https://github.com/meteor/meteor/pull/10772#issuecomment-553517459
The assertion in tools/fs/optimistic.ts was failing if I passed a relative
path for --test-app-path, and passing the path as a second argument when
calling assert made it easier to tell what was going on, so I decided to
keep that change.
Now that files.rename uses Promise.prototype.await on Windows, it's
important to be sure it never gets called outside of a Fiber. Though we
don't run our full test suite on Windows, we can validate this expectation
by wrapping files.rename on all platforms. This commit should be reverted
once the validation is complete.
Falling back to a full recursive copy was MUCH more expensive than waiting
a short amount of time before retrying the rename.
This aligns with the way graceful-fs handles EPERM and EACCES errors on
Windows: https://www.npmjs.com/package/graceful-fs#improvements-over-fs-module
Using fs.writeFileSync in a serial style becomes especially costly when
we're writing a lot of files. In a recent profiling exercise I did on
Windows, nearly 80% of the time taken by Builder#_copyDirectory was spent
just closing the written files. By using the asynchronous fs.writeFile
function, we should be able to parallelize at least some of this work, and
await all the promises at the very end of copying the directory.
* Avoid modifying unibuild.watchSet in PackageSourceBatch._watchOutputFiles.
Should fix#10736 by preventing old hashes from remaining in
unibuild.watchSet, which was sometimes causing isUpToDate to fail
immediately upon restart.
* Regression test for issue #10736.
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.
