From 86ee5c9477b8403fd424929db842d8b3b0e3ba9b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B6nke=20Ludwig?= Date: Fri, 8 May 2020 18:30:22 +0200 Subject: [PATCH] Remove FileChange.isDirectory. isDir can be a huge performance issue when watching a network based directory and can effectively block the thread almost completely in case of frequent file changes. This does mean than high-level code now needs to perform the check manually, if required, and the free information provided by inotify goes unused. --- source/eventcore/driver.d | 7 ----- source/eventcore/drivers/posix/watchers.d | 11 ++++--- source/eventcore/drivers/winapi/watchers.d | 5 +--- tests/0-dirwatcher-rec.d | 34 +++++++++++++++------- tests/0-dirwatcher.d | 8 ++++- 5 files changed, 37 insertions(+), 28 deletions(-) diff --git a/source/eventcore/driver.d b/source/eventcore/driver.d index 6bd00e2..fe0d6dd 100644 --- a/source/eventcore/driver.d +++ b/source/eventcore/driver.d @@ -1093,13 +1093,6 @@ struct FileChange { /// Name of the changed file const(char)[] name; - - /** Determines if the changed entity is a file or a directory. - - Note that depending on the platform this may not be accurate for - `FileChangeKind.removed`. - */ - bool isDirectory; } /** Describes a spawned process diff --git a/source/eventcore/drivers/posix/watchers.d b/source/eventcore/drivers/posix/watchers.d index ac2b753..30d7233 100644 --- a/source/eventcore/drivers/posix/watchers.d +++ b/source/eventcore/drivers/posix/watchers.d @@ -150,7 +150,6 @@ final class InotifyEventDriverWatchers(Events : EventDriverEvents) : EventDriver ch.baseDirectory = m_watches[id].basePath; ch.directory = subdir; - ch.isDirectory = (ev.mask & IN_ISDIR) != 0; ch.name = name; addRef(id); // assure that the id doesn't get invalidated until after the callback auto cb = m_loop.m_fds[id].watcher.callback; @@ -487,9 +486,9 @@ final class PollEventDriverWatchers(Events : EventDriverEvents) : EventDriverWat @property size_t entryCount() const { return m_entryCount; } - private void addChange(FileChangeKind kind, Key key, bool is_dir) + private void addChange(FileChangeKind kind, Key key) { - m_onChange(FileChange(kind, m_basePath, key.parent ? key.parent.path : "", key.name, is_dir)); + m_onChange(FileChange(kind, m_basePath, key.parent ? key.parent.path : "", key.name)); } private void scan(bool generate_changes) @@ -506,12 +505,12 @@ final class PollEventDriverWatchers(Events : EventDriverEvents) : EventDriverWat foreach (e; m_entries.byKeyValue) { if (!e.key.parent || Key(e.key.parent.parent, e.key.parent.name) !in m_entries) { if (generate_changes) - addChange(FileChangeKind.removed, e.key, e.value.isDir); + addChange(FileChangeKind.removed, e.key); } } foreach (e; added) - addChange(FileChangeKind.added, Key(e.parent, e.name), e.isDir); + addChange(FileChangeKind.added, Key(e.parent, e.name)); swap(m_entries, new_entries); m_entryCount = ec; @@ -539,7 +538,7 @@ final class PollEventDriverWatchers(Events : EventDriverEvents) : EventDriverWat } else { if ((*pe).size != de.size || (*pe).lastChange != modified_time) { if (generate_changes) - addChange(FileChangeKind.modified, key, (*pe).isDir); + addChange(FileChangeKind.modified, key); (*pe).size = de.size; (*pe).lastChange = modified_time; } diff --git a/source/eventcore/drivers/winapi/watchers.d b/source/eventcore/drivers/winapi/watchers.d index 679d15f..ec58b00 100644 --- a/source/eventcore/drivers/winapi/watchers.d +++ b/source/eventcore/drivers/winapi/watchers.d @@ -193,10 +193,7 @@ final class WinAPIEventDriverWatchers : EventDriverWatchers { ch.directory = dirName(path); if (ch.directory == ".") ch.directory = ""; ch.name = baseName(path); - try ch.isDirectory = isDir(fullpath); - catch (Exception e) {} // FIXME: can happen if the base path is relative and the CWD has changed - if (ch.kind != FileChangeKind.modified || !ch.isDirectory) - slot.callback(id, ch); + slot.callback(id, ch); if (fni.NextEntryOffset == 0 || !slot.callback) break; } diff --git a/tests/0-dirwatcher-rec.d b/tests/0-dirwatcher-rec.d index 880a69c..a52eab2 100644 --- a/tests/0-dirwatcher-rec.d +++ b/tests/0-dirwatcher-rec.d @@ -8,7 +8,7 @@ import eventcore.core; import eventcore.internal.utils : print; import core.thread : Thread; import core.time : Duration, MonoTime, msecs; -import std.file : exists, remove, rename, rmdirRecurse, mkdir; +import std.file : exists, remove, rename, rmdirRecurse, mkdir, isDir; import std.format : format; import std.functional : toDelegate; import std.path : baseName, buildPath, dirName; @@ -97,6 +97,20 @@ void expectChange(FileChange ch, bool expect_change) assert(!expect_change, format("Got no change, expected %s.", ch)); return; } + + // ignore different directory modification notifications on Windows as + // opposed to the other systems + while (pendingChanges.length) { + auto pch = pendingChanges[0]; + if (pch.kind == FileChangeKind.modified) { + auto p = buildPath(pch.baseDirectory, pch.directory, pch.name); + if (!exists(p) || isDir(p)) { + pendingChanges = pendingChanges[1 .. $]; + continue; + } + } + break; + } } assert(expect_change, "Got change although none was expected."); @@ -119,43 +133,43 @@ void testFile(string name, bool expect_change = true) { print("test %s CREATE %s", name, expect_change); auto fil = File(buildPath(testDir, name), "wt"); - expectChange(fchange(FileChangeKind.added, name, false), expect_change); + expectChange(fchange(FileChangeKind.added, name), expect_change); print("test %s MODIFY %s", name, expect_change); fil.write("test"); fil.close(); - expectChange(fchange(FileChangeKind.modified, name, false), expect_change); + expectChange(fchange(FileChangeKind.modified, name), expect_change); print("test %s DELETE %s", name, expect_change); remove(buildPath(testDir, name)); - expectChange(fchange(FileChangeKind.removed, name, false), expect_change); + expectChange(fchange(FileChangeKind.removed, name), expect_change); } void testCreateDir(string name, bool expect_change = true) { print("test %s CREATEDIR %s", name, expect_change); mkdir(buildPath(testDir, name)); - expectChange(fchange(FileChangeKind.added, name, true), expect_change); + expectChange(fchange(FileChangeKind.added, name), expect_change); } void testRemoveDir(string name, bool expect_change = true) { print("test %s DELETEDIR %s", name, expect_change); rmdirRecurse(buildPath(testDir, name)); - expectChange(fchange(FileChangeKind.removed, name, true), expect_change); + expectChange(fchange(FileChangeKind.removed, name), expect_change); } void testRename(string from, string to, bool expect_change = true) { print("test %s RENAME %s %s", from, to, expect_change); rename(buildPath(testDir, from), buildPath(testDir, to)); - expectChange(fchange(FileChangeKind.removed, from, true), expect_change); - expectChange(fchange(FileChangeKind.added, to, true), expect_change); + expectChange(fchange(FileChangeKind.removed, from), expect_change); + expectChange(fchange(FileChangeKind.added, to), expect_change); } -FileChange fchange(FileChangeKind kind, string name, bool is_dir) +FileChange fchange(FileChangeKind kind, string name) { auto dn = dirName(name); if (dn == ".") dn = ""; - return FileChange(kind, testDir, dn, baseName(name), is_dir); + return FileChange(kind, testDir, dn, baseName(name)); } diff --git a/tests/0-dirwatcher.d b/tests/0-dirwatcher.d index a7ec09a..5cc21d3 100644 --- a/tests/0-dirwatcher.d +++ b/tests/0-dirwatcher.d @@ -5,8 +5,9 @@ module test; import eventcore.core; +import std.file : exists, isDir, mkdir, remove, rmdirRecurse; +import std.path : buildPath; import std.stdio : File, writefln; -import std.file : exists, mkdir, remove, rmdirRecurse; import core.time : Duration, msecs; bool s_done; @@ -23,6 +24,11 @@ void main() scope (exit) rmdirRecurse(testDir); auto id = eventDriver.watchers.watchDirectory(testDir, false, (id, ref change) { + try { + if (change.kind == FileChangeKind.modified && isDir(buildPath(change.baseDirectory, change.directory, change.name))) + return; + } catch (Exception e) assert(false, e.msg); + switch (s_cnt++) { default: import std.conv : to;