From dea305ca4a8b199e15975daba2dfbcd84fd6dfef Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Tue, 5 Nov 2019 10:53:17 -0500 Subject: [PATCH] Set a higher LRU cache size for makeCheapPathFunction entries. In PR #10720, we introduced the makeCheapPathFunction in an effort to reduce the caching overhead for very frequently called (and already pretty quick) operations like files.stat. However, the default maximum LRU cache size of Math.pow(2, 16) can cause quite a bit of cache eviction churn for large applications, which @veered has identified as a potential source of build performance problems. By setting the maximum cache size to Math.pow(2, 20) instead, I am no longer seeing any files.stat calls in the profiling output for rebuilding a large internal app, saving several seconds of rebuild time. The obvious downside is that this cache might accumulate more memory over time, which is why I didn't just set the max to Infinity, though that might be a viable option if the total set of paths ever stat'd is small enough to fit into the available memory. In the future, I hope to find ways of managing LRU cache size that respond to actual memory pressure (relative to available memory), rather than pruning the cache after an arbitrary numeric threshold is reached. --- tools/fs/optimistic.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tools/fs/optimistic.ts b/tools/fs/optimistic.ts index c8a8d40b54..e351092178 100644 --- a/tools/fs/optimistic.ts +++ b/tools/fs/optimistic.ts @@ -229,6 +229,11 @@ function makeCheapPathFunction( return pathFunction; } const wrapper = wrap(pathFunction, { + // The maximum LRU cache size is Math.pow(2, 16) by default, but it's + // important to prevent eviction churn for very-frequently-called + // functions like optimisticStatOrNull. While it's tempting to set + // this limit to Infinity, increasing it by 16x comes close enough. + max: Math.pow(2, 20), subscribe(path) { let watcher: SafeWatcher | null = watch( path,