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.
I noticed recently that progress messages were no longer displayed (though
the ASCII spinner was still animating). I bisected the problem to the
progress.ts conversion in #10650, and I attempted to reproduce the
conversion, step by step. The problem ended up being the renaming of
._title to .title, which was a mistake because external code accesses
progress._title. Once I updated those invasive access points, progress
messages began displaying once again.
Partial alternative to using json5, which we tried in commit
5124cb495c, which was recently reverted due
to performance concerns: 0e19c365a8.
Should solve the specific problem reported in #10688.
This reverts commit 5124cb495c.
Perhaps unsurprisingly, JSON5 is much slower than JSON, so much so that
"other optimisticReadJsonOrNull" has become the single most expensive leaf
when building one of our internal applications. We will have to find
another solution.
This is important so that tools like VSCode will pick up the correct
version of the TypeScript SDK, rather than whatever default version was
installed with VSCode.
Just as developers can run
meteor create --react my-react-app
meteor create --minimal my-minimal-app
meteor create --bare my-bare-app
meteor create --full my-full-app
they can now run
meteor create --typescript my-typescript-app
to produce a new TypeScript application, based on the --react application,
configured using a recommended tsconfig.json file.
This app represents the current best/simplest-known way to set up a
Meteor-compatible TypeScript application, but it is still very much a work
in progress. Please feel free to submit pull requests to improve it, or
create issues to discuss how it should work.
For example, the community-maintained @types/meteor package covers all the
core packages used by this starter application, but it has not been
updated in a while, so there will no doubt be meteor/* packages with
missing types. In future versions, Meteor should ideally generate the
appropriate .d.ts files from TypeScript package source code, so that no
separate @types/meteor/* declarations need to be maintained.
This function determines whether the optimistic caching system should
subscribe to file change notifications for a given path.
Note that Meteor has other ways of watching for file changes (e.g. the
WatchSet abstraction), as well as other mechanisms for invalidating cached
results, so time and resources can often be saved by returning false here,
if we know that the path will be watched in other ways.
Improvements:
- Add METEOR_PROFILE instrumentation to the shouldWatch implementation.
- Stop exporting shouldWatch, since it is no longer used anywhere else in
the codebase.
- Return false early to avoid watching files inside the .meteor directory,
a significant shortcut.
- Allow watching files in application node_modules directories that are
not located directly in the root application directory (closes#10664).
Because json5 is more tolerant of non-standards-compliant input, it has no
trouble with \t tab characters outside of string literals, so this change
should fix issue #10688.
Note that json5 is already installed in dev_bundle/lib/node_modules/json5,
because it is a dependency of @babel/core, so we don't need to rebuild the
dev bundle immediately, though it would be a good idea to do so before the
next beta/RC release.
The previous implementation simply avoided calling watchSet.addFile for
potentially unused files, trusting that addFile would be called later if
the file was eventually used. However, this strategy left the contents of
watchSet.files incomplete for tasks such as IsopackCache._checkUpToDate,
which require full information about all files, even the ones that might
not be used by the bundle. The new strategy maintains metadata about
potentially unused files in a separate data structure, which will be
merged/cloned/serialized/deserialized along with other WatchSet data.
Sometimes, before dev_bundle/bin/.meteor-commands.json has been written, a
command like `./meteor npx tsc` can fail because `npx` is not blessed in
the same way `node` and `npm` are. Since `npx` is a core tool that we want
folks to be able to use without confusion, I think it makes sense to
include it in the list of known subcommands.
Unfortunately, this conversion triggered an error due to one of the
shortcomings of the Babel implementation of TypeScript:
SyntaxError: /tools/tool-env/profile.ts: Namespace not marked type-only declare. Non-declarative namespaces are only supported experimentally in Babel. To enable and review caveats see: https://babeljs.io/docs/en/babel-plugin-transform-typescript
278 | }
279 |
> 280 | export namespace Profile {
| ^
281 | export let enabled = !! process.env.METEOR_PROFILE;
282 |
283 | export function time<TResult>(bucket: string, f: () => TResult) {
at File.buildCodeFrameError (/Users/ben/meteor/dev_bundle/lib/node_modules/@babel/core/lib/transformation/file/file.js:261:12)
at transpileNamespace (/Users/ben/meteor/dev_bundle/lib/node_modules/@babel/plugin-transform-typescript/lib/namespace.js:25:25)
at PluginPass.TSModuleDeclaration (/Users/ben/meteor/dev_bundle/lib/node_modules/@babel/plugin-transform-typescript/lib/index.js:271:32)
at newFn (/Users/ben/meteor/dev_bundle/lib/node_modules/@babel/traverse/lib/visitors.js:193:21)
at NodePath._call (/Users/ben/meteor/dev_bundle/lib/node_modules/@babel/traverse/lib/path/context.js:53:20)
at NodePath.call (/Users/ben/meteor/dev_bundle/lib/node_modules/@babel/traverse/lib/path/context.js:40:17)
at NodePath.visit (/Users/ben/meteor/dev_bundle/lib/node_modules/@babel/traverse/lib/path/context.js:88:12)
at TraversalContext.visitQueue (/Users/ben/meteor/dev_bundle/lib/node_modules/@babel/traverse/lib/context.js:118:16)
at TraversalContext.visitMultiple (/Users/ben/meteor/dev_bundle/lib/node_modules/@babel/traverse/lib/context.js:85:17)
at TraversalContext.visit (/Users/ben/meteor/dev_bundle/lib/node_modules/@babel/traverse/lib/context.js:144:19)
