-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Analyzer training runs are broken on Fuchsia #34616
Comments
Server stores a session id on disk for the purposes of logging information (internal users only). It puts the file in the user's home directory: final home = io.Platform.isWindows ? env['LOCALAPPDATA'] : env['HOME']; I'm guessing that on Fuchsia either there is no home directory or this isn't the right way to get the path. Can you tell me which case I'm looking at? |
|
There won't be a home directory during the Fuchsia build, and it would be best not to touch the environment at all, really even in the non-Fuchsia case. It could potentially be a source of non-hermetic-ness. |
We can fix the code so that it doesn't break when there's no home directory, but the training program we use writes to disk, so that might be a bigger issue. We do that because during normal use the analysis server uses the disk as a cache. The training analyzes the same set of source files twice: once when the disk cache is empty (which fills the cache) and once when it is full. This causes us to exercise two code paths that are important for optimal performance. I'm not sure how we want to deal with that. Ideas? |
…pdate docs. Disable broken analyzer training runs. Bug: #34616 Change-Id: Ib2bb022f5b095bcd29451741aa4ed557d2f78c17 Reviewed-on: https://dart-review.googlesource.com/77228 Reviewed-by: Zach Anderson <zra@google.com> Commit-Queue: Ryan Macnak <rmacnak@google.com>
Can you accept the cache directory as a command line argument? |
Yes, using |
So, to clarify: That flag already exists, and if it is set, it will bypass touching $HOME? |
No. There are two different sets of data being cached. There is a UUID used to represent a user to a reporting mechanism used only for internal users (which is what's looking for $HOME), and there is a set of data per library representing the results of analyzing the library. The flag controls the location of the latter, but not the former. |
This should almost certainly be turned off during a training run during the build if it is not already. |
I think that https://dart-review.googlesource.com/c/sdk/+/77422 should fix the issue, but I don't have any way to test it. Please let me know if the problem persists or if you need any other changes. |
The analyzer snapshot is currently "trained" with `--help`, i.e. not really trained at all. Doing some archaeology, it actually used to be trained, but it was disabled in https://dart-review.git.corp.google.com/c/sdk/+/77228 because of Fuchsia issues (#34616). Although the troublesome stuff was perhaps fixed in https://dart-review.git.corp.google.com/c/sdk/+/77422 it was seemingly never re-enabled. That was 2018. This CL re-enables it, taking an initial uncached analysis of dart2js to ~51% (~9.5s vs ~18.6s), of the analyzer to ~75% (~22.2s vs ~29.6) and of the front_end to ~50% (~7.7s vs ~15.2s). This won’t change the peak performance of the analyzer, but will certainly - as seen above - make it faster when first loaded. Change-Id: I613e6a5b52d00da15948cf936f20ea36ad28818e Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/269683 Reviewed-by: Ryan Macnak <rmacnak@google.com> Commit-Queue: Jens Johansen <jensj@google.com>
The text was updated successfully, but these errors were encountered: