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.
This commit is contained in:
Sönke Ludwig 2020-05-08 18:30:22 +02:00
parent d7657b54e8
commit 86ee5c9477
5 changed files with 37 additions and 28 deletions

View file

@ -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

View file

@ -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;
}

View file

@ -193,9 +193,6 @@ 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);
if (fni.NextEntryOffset == 0 || !slot.callback) break;
}

View file

@ -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));
}

View file

@ -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;