Skip to content

Script startup latency improvement #11660

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

Closed
wants to merge 4 commits into from
Closed

Script startup latency improvement #11660

wants to merge 4 commits into from

Conversation

philwalk
Copy link
Contributor

@philwalk philwalk commented Mar 8, 2021

When launching a scala3 script, there are two calls to dist/bin/common, the first from within dist/bin/scala and the 2nd when dist/bin/scala calls dist/bin/scalac.

It turns out that the avoidable 2nd call is quite expensive in terms of what it adds to script startup latency. Here are the reductions in startup time when running in cygwin and in WSL Linux:

cygwin:    837 mSec
WSL Linux:  66 mSec

The numbers are the difference in startup latency for calling a hello-world.sc script with and without the changes in this PR, averaged over 100 runs.

The categories of change in the PR are:

  • exporting values initialized by dist/bin/common and required by scalac
  • modification of dist/bin/scalac to conditionally source dist/bin/common
  • changes to avoid spurious unset variable errors when testing with: set -o nounset
  • fail early if a script file doesn't exist

The third change is exemplified by line 114 in dist/bin/common:

-if [ -n "$CYGPATHCMD" ]; then
+if [ -n "${CYGPATHCMD-}" ]; then

@philwalk philwalk closed this Mar 8, 2021
@philwalk
Copy link
Contributor Author

philwalk commented Mar 8, 2021

Unintentionally included the changes from #11633, I'll have to re-create this PR later.

@philwalk philwalk reopened this Mar 8, 2021
@philwalk philwalk closed this Mar 8, 2021
@philwalk

This comment has been minimized.

@philwalk philwalk reopened this Mar 8, 2021
@griggt
Copy link
Contributor

griggt commented Mar 8, 2021

Just an FYI, I've noticed that the act of re-opening a PR launches a fresh CI run. There are 3 jobs now running in CI for this PR:

https://github.com/lampepfl/dotty/actions/runs/633879155
https://github.com/lampepfl/dotty/actions/runs/633879159
https://github.com/lampepfl/dotty/actions/runs/633891730

@philwalk
Copy link
Contributor Author

@griggt - good to know, will avoid this in the future.

@philwalk
Copy link
Contributor Author

This PR needs to be re-created from scratch, to avoid possible problems with #11633

@philwalk
Copy link
Contributor Author

This PR is superceded by #11698

@philwalk philwalk deleted the script-startup-latency-improvement branch March 11, 2021 12:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants