Skip to content

Commit e8010ae

Browse files
sokrasokra
authoredAug 27, 2020
Merge pull request #175 from webpack/bugfix/plan-merging
fix bugs in recursive watching
·
v2.4.2v2.0.0-beta.15
2 parents 9ec3868 + dc01813 commit e8010ae

File tree

3 files changed

+175
-13
lines changed

3 files changed

+175
-13
lines changed
 

‎lib/reducePlan.js

+14-8
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,9 @@ module.exports = (plan, limit) => {
5252
value: undefined
5353
};
5454
treeMap.set(parentPath, parent);
55+
node.parent = parent;
5556
} else {
57+
node.parent = parent;
5658
if (parent.children === undefined) {
5759
parent.children = [node];
5860
} else {
@@ -68,12 +70,13 @@ module.exports = (plan, limit) => {
6870
// Reduce until limit reached
6971
while (currentCount > limit) {
7072
// Select node that helps reaching the limit most effectively without overmerging
71-
const overLimit = limit - currentCount;
73+
const overLimit = currentCount - limit;
7274
let bestNode = undefined;
7375
let bestCost = Infinity;
7476
for (const node of treeMap.values()) {
75-
if (node.entries <= 1 || !node.children) continue;
76-
if (node.children.length <= 1) continue;
77+
if (node.entries <= 1 || !node.children || !node.parent) continue;
78+
if (node.children.length === 0) continue;
79+
if (node.children.length === 1 && !node.value) continue;
7780
// Try to select the node with has just a bit more entries than we need to reduce
7881
// When just a bit more is over 30% over the limit,
7982
// also consider just a bit less entries then we need to reduce
@@ -108,11 +111,12 @@ module.exports = (plan, limit) => {
108111
}
109112
// Write down new plan
110113
const newPlan = new Map();
111-
for (const node of treeMap.values()) {
112-
if (!node.active) continue;
114+
for (const rootNode of treeMap.values()) {
115+
if (!rootNode.active) continue;
113116
const map = new Map();
114-
const queue = new Set([node]);
117+
const queue = new Set([rootNode]);
115118
for (const node of queue) {
119+
if (node.active && node !== rootNode) continue;
116120
if (node.value) {
117121
if (Array.isArray(node.value)) {
118122
for (const item of node.value) {
@@ -123,10 +127,12 @@ module.exports = (plan, limit) => {
123127
}
124128
}
125129
if (node.children) {
126-
for (const child of node.children) queue.add(child);
130+
for (const child of node.children) {
131+
queue.add(child);
132+
}
127133
}
128134
}
129-
newPlan.set(node.filePath, map);
135+
newPlan.set(rootNode.filePath, map);
130136
}
131137
return newPlan;
132138
};

‎test/Assumption.js

+145-1
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,10 @@ var TestHelper = require("./helpers/TestHelper");
99
var fixtures = path.join(__dirname, "fixtures");
1010
var testHelper = new TestHelper(fixtures);
1111

12+
const IS_OSX = require("os").platform() === "darwin";
13+
const IS_WIN = require("os").platform() === "win32";
14+
const SUPPORTS_RECURSIVE_WATCHING = IS_OSX || IS_WIN;
15+
1216
describe("Assumption", function() {
1317
this.timeout(10000);
1418
var watcherToClose = null;
@@ -145,6 +149,76 @@ describe("Assumption", function() {
145149
}
146150
});
147151

152+
if (SUPPORTS_RECURSIVE_WATCHING) {
153+
it("should have a file system with correct mtime behavior (fs.watch recursive)", function(done) {
154+
this.timeout(20000);
155+
testHelper.file("a");
156+
var i = 60;
157+
var count = 60;
158+
var before;
159+
var after;
160+
var minDiffBefore = +Infinity;
161+
var maxDiffBefore = -Infinity;
162+
var sumDiffBefore = 0;
163+
var minDiffAfter = +Infinity;
164+
var maxDiffAfter = -Infinity;
165+
var sumDiffAfter = 0;
166+
var watcher = (watcherToClose = fs.watch(fixtures, { recursive: true }));
167+
testHelper.tick(100, function() {
168+
watcher.on("change", function(type, filename) {
169+
const s = fs.statSync(path.join(fixtures, filename));
170+
if (before && after) {
171+
var diffBefore = +s.mtime - before;
172+
if (diffBefore < minDiffBefore) minDiffBefore = diffBefore;
173+
if (diffBefore > maxDiffBefore) maxDiffBefore = diffBefore;
174+
sumDiffBefore += diffBefore;
175+
var diffAfter = +s.mtime - after;
176+
if (diffAfter < minDiffAfter) minDiffAfter = diffAfter;
177+
if (diffAfter > maxDiffAfter) maxDiffAfter = diffAfter;
178+
sumDiffAfter += diffAfter;
179+
before = after = undefined;
180+
if (i-- === 0) {
181+
afterMeasure();
182+
} else {
183+
testHelper.tick(100, checkMtime);
184+
}
185+
}
186+
});
187+
testHelper.tick(100, checkMtime);
188+
});
189+
190+
function checkMtime() {
191+
before = Date.now();
192+
testHelper.file("a");
193+
after = Date.now();
194+
}
195+
196+
function afterMeasure() {
197+
console.log(
198+
"mtime fs.watch({ recursive: true }) accuracy (before): [" +
199+
minDiffBefore +
200+
" ; " +
201+
maxDiffBefore +
202+
"] avg " +
203+
Math.round(sumDiffBefore / count)
204+
);
205+
console.log(
206+
"mtime fs.watch({ recursive: true }) accuracy (after): [" +
207+
minDiffAfter +
208+
" ; " +
209+
maxDiffAfter +
210+
"] avg " +
211+
Math.round(sumDiffAfter / count)
212+
);
213+
minDiffBefore.should.be.aboveOrEqual(-2000);
214+
maxDiffBefore.should.be.below(2000);
215+
minDiffAfter.should.be.aboveOrEqual(-2000);
216+
maxDiffAfter.should.be.below(2000);
217+
done();
218+
}
219+
});
220+
}
221+
148222
it("should not fire events in subdirectories", function(done) {
149223
testHelper.dir("watch-test-directory");
150224
testHelper.tick(500, () => {
@@ -166,7 +240,77 @@ describe("Assumption", function() {
166240
});
167241
});
168242

169-
if (require("os").platform() !== "darwin") {
243+
if (SUPPORTS_RECURSIVE_WATCHING) {
244+
it("should fire events in subdirectories (recursive)", function(done) {
245+
testHelper.dir("watch-test-directory");
246+
testHelper.file("watch-test-directory/watch-test-file");
247+
testHelper.file("watch-test-directory/existing-file");
248+
testHelper.tick(500, () => {
249+
var watcher = (watcherToClose = fs.watch(fixtures, {
250+
recursive: true
251+
}));
252+
const events = [];
253+
watcher.once("change", () => {
254+
testHelper.tick(1000, function() {
255+
events.should.matchAny(/watch-test-directory[/\\]watch-test-file/);
256+
done();
257+
});
258+
});
259+
watcher.on("change", function(type, filename) {
260+
events.push(filename);
261+
});
262+
watcher.on("error", function(err) {
263+
done(err);
264+
done = function() {};
265+
});
266+
testHelper.tick(500, function() {
267+
testHelper.file("watch-test-directory/watch-test-file");
268+
});
269+
});
270+
});
271+
272+
it("should allow to create/close/create recursive watchers", function(done) {
273+
testHelper.dir("watch-test-directory");
274+
testHelper.file("watch-test-directory/watch-test-file");
275+
testHelper.file("watch-test-directory/existing-file");
276+
testHelper.tick(500, () => {
277+
watcherToClose = fs.watch(fixtures, {
278+
recursive: true
279+
});
280+
watcherToClose.close();
281+
watcherToClose = fs.watch(fixtures, {
282+
recursive: true
283+
});
284+
watcherToClose.close();
285+
watcherToClose = fs.watch(fixtures, {
286+
recursive: true
287+
});
288+
watcherToClose.close();
289+
var watcher = (watcherToClose = fs.watch(fixtures, {
290+
recursive: true
291+
}));
292+
const events = [];
293+
watcher.once("change", () => {
294+
testHelper.tick(1000, function() {
295+
events.should.matchAny(/watch-test-directory[/\\]watch-test-file/);
296+
done();
297+
});
298+
});
299+
watcher.on("change", function(type, filename) {
300+
events.push(filename);
301+
});
302+
watcher.on("error", function(err) {
303+
done(err);
304+
done = function() {};
305+
});
306+
testHelper.tick(500, function() {
307+
testHelper.file("watch-test-directory/watch-test-file");
308+
});
309+
});
310+
});
311+
}
312+
313+
if (!IS_OSX) {
170314
it("should detect removed directory", function(done) {
171315
testHelper.dir("watch-test-dir");
172316
testHelper.tick(() => {

‎test/ManyWatchers.js

+16-4
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,16 @@ const fixtures = path.join(__dirname, "fixtures");
1111
const testHelper = new TestHelper(fixtures);
1212

1313
describe("ManyWatchers", function() {
14-
this.timeout(240000);
14+
this.timeout(600000);
1515
beforeEach(testHelper.before);
1616
afterEach(testHelper.after);
1717

1818
it("should watch more than 4096 directories", done => {
19+
console.time("creating files");
20+
// windows is very slow in creating so many files
21+
// this can take about 1 minute
1922
const files = [];
20-
for (let i = 1; i <= 5000; i++) {
23+
for (let i = 1; i < 5000; i++) {
2124
let highBit = 1;
2225
let j = i;
2326
while (j > 1) {
@@ -38,24 +41,33 @@ describe("ManyWatchers", function() {
3841
files.push(path.join(fixtures, dir, "file2"));
3942
}
4043
}
41-
testHelper.tick(1000, () => {
44+
testHelper.file("file");
45+
files.push(path.join(fixtures, "file"));
46+
console.timeEnd("creating files");
47+
testHelper.tick(10000, () => {
4248
const w = new Watchpack({
4349
aggregateTimeout: 1000
4450
});
4551
w.on("aggregated", function(changes) {
52+
console.timeEnd("detecting change event");
4653
Array.from(changes).should.be.eql([
4754
path.join(fixtures, "4096/900/file")
4855
]);
4956
w.close();
5057
done();
5158
});
52-
for (let i = 100; i < files.length; i += 987) {
59+
console.time("creating/closing watchers");
60+
// MacOS is very slow in creating and destroying watchers
61+
// This can take about 2 minutes
62+
for (let i = 100; i < files.length; i += 2432) {
5363
for (let j = 0; j < files.length - i; j += 987) {
5464
w.watch({ files: files.slice(j, j + i) });
5565
}
5666
}
5767
w.watch({ files });
68+
console.timeEnd("creating/closing watchers");
5869
testHelper.tick(10000, () => {
70+
console.time("detecting change event");
5971
testHelper.file("4096/900/file");
6072
});
6173
});

0 commit comments

Comments
 (0)
Please sign in to comment.