sync/watch: reduce memory usage
Currently we get a different inotify handle for every file in the
synclist, and register all of them with sd_event. A single fd will
work and saves memory in libsystemd and the kernel, so do that instead.
This in turn allows the fileMap structure to be simplified down to a
basic map of watch descriptor / filename pairs.
Remove redundant calls to both close and inotify_rm_watch - let the
kernel do the work for us. From the inotify man page:
When all file descriptors referring to an inotify instance have been
closed (using close(2)), the underlying object and its resources are
freed for reuse by the kernel; all associated watches are auto‐
matically freed.
Change-Id: Ia63cb667cdf41c171276a0351d95347a54578f2f
Signed-off-by: Brad Bishop <bradleyb@fuzziesquirrel.com>
Signed-off-by: Adriana Kobylak <anoo@us.ibm.com>
diff --git a/sync_watch.cpp b/sync_watch.cpp
index 88ccd89..bb2a860 100644
--- a/sync_watch.cpp
+++ b/sync_watch.cpp
@@ -21,41 +21,38 @@
void SyncWatch::addInotifyWatch(const fs::path& path)
{
- auto fd = inotify_init1(IN_NONBLOCK);
- if (-1 == fd)
- {
- log<level::ERR>("inotify_init1 failed", entry("ERRNO=%d", errno),
- entry("FILENAME=%s", path.c_str()));
- return;
- }
-
- auto wd = inotify_add_watch(fd, path.c_str(), IN_CLOSE_WRITE | IN_DELETE);
+ auto wd =
+ inotify_add_watch(inotifyFd, path.c_str(), IN_CLOSE_WRITE | IN_DELETE);
if (-1 == wd)
{
log<level::ERR>("inotify_add_watch failed", entry("ERRNO=%d", errno),
entry("FILENAME=%s", path.c_str()));
- close(fd);
return;
}
- auto rc = sd_event_add_io(&loop, nullptr, fd, EPOLLIN, callback, this);
- if (0 > rc)
- {
- log<level::ERR>("failed to add to event loop", entry("RC=%d", rc),
- entry("FILENAME=%s", path.c_str()));
- inotify_rm_watch(fd, wd);
- close(fd);
- return;
- }
-
- fileMap[fd].insert(std::make_pair(wd, fs::path(path)));
+ fileMap[wd] = fs::path(path);
}
SyncWatch::SyncWatch(sd_event& loop,
std::function<int(int, fs::path&)> syncCallback) :
- syncCallback(syncCallback),
- loop(loop)
+ inotifyFd(-1),
+ syncCallback(syncCallback), loop(loop)
{
+ auto fd = inotify_init1(IN_NONBLOCK);
+ if (-1 == fd)
+ {
+ log<level::ERR>("inotify_init1 failed", entry("ERRNO=%d", errno));
+ return;
+ }
+ inotifyFd = fd;
+
+ auto rc = sd_event_add_io(&loop, nullptr, fd, EPOLLIN, callback, this);
+ if (0 > rc)
+ {
+ log<level::ERR>("failed to add to event loop", entry("RC=%d", rc));
+ return;
+ }
+
auto syncfile = fs::path(SYNC_LIST_DIR_PATH) / SYNC_LIST_FILE_NAME;
if (fs::exists(syncfile))
{
@@ -70,13 +67,9 @@
SyncWatch::~SyncWatch()
{
- for (const auto& fd : fileMap)
+ if (inotifyFd != -1)
{
- for (const auto& wd : fd.second)
- {
- inotify_rm_watch(fd.first, wd.first);
- }
- close(fd.first);
+ close(inotifyFd);
}
}
@@ -102,32 +95,29 @@
{
auto event = reinterpret_cast<inotify_event*>(&buffer[offset]);
- // fileMap<fd, std::map<wd, path>>
- auto it1 = syncWatch->fileMap.find(fd);
- if (it1 != syncWatch->fileMap.end())
+ // Watch was removed, re-add it if file still exists.
+ if (event->mask & IN_IGNORED)
{
- auto it2 = it1->second.begin();
-
- // Watch was removed, re-add it if file still exists.
- if (event->mask & IN_IGNORED)
+ if (fs::exists(syncWatch->fileMap[event->wd]))
{
- if (fs::exists(it2->second))
- {
- syncWatch->addInotifyWatch(it2->second);
- }
- else
- {
- log<level::INFO>("The inotify watch was removed",
- entry("FILENAME=%s", it2->second.c_str()));
- }
- return 0;
+ syncWatch->addInotifyWatch(syncWatch->fileMap[event->wd]);
}
-
- auto rc = syncWatch->syncCallback(event->mask, it2->second);
- if (rc)
+ else
{
- return rc;
+ log<level::INFO>(
+ "The inotify watch was removed",
+ entry("FILENAME=%s",
+ (syncWatch->fileMap[event->wd]).c_str()));
}
+ return 0;
+ }
+
+ // fileMap<wd, path>
+ auto rc =
+ syncWatch->syncCallback(event->mask, syncWatch->fileMap[event->wd]);
+ if (rc)
+ {
+ return rc;
}
offset += offsetof(inotify_event, name) + event->len;
diff --git a/sync_watch.hpp b/sync_watch.hpp
index 3ad5afd..126c50c 100644
--- a/sync_watch.hpp
+++ b/sync_watch.hpp
@@ -63,7 +63,8 @@
/** @brief Map of file descriptors, watch descriptors, and file paths */
using fd = int;
using wd = int;
- std::map<fd, std::map<wd, fs::path>> fileMap;
+ fd inotifyFd;
+ std::map<wd, fs::path> fileMap;
/** @brief The callback function for processing the inotify event */
std::function<int(int, fs::path&)> syncCallback;