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

Optims #853

Merged
merged 6 commits into from Aug 29, 2018

Conversation

Projects
None yet
1 participant
@alexarchambault
Collaborator

alexarchambault commented Aug 23, 2018

In a few words, this PR:

  • tries to have Ammonite not unnecessarily run stty commands before / after each line of code, but only when Ammonite starts and exits instead (first commit),
  • tries to have Ammonite not discard its compiler instance after almost each line, but only when new JARs are added to the classpath (actual JARs, not just compilation class files, second and third commits).

(of course, github tangled with the commits ordering…)

@alexarchambault

This comment has been minimized.

Collaborator

alexarchambault commented Aug 23, 2018

Results of a quick benchmark:
amm benchmark full 2

duration in milliseconds, roughly of ~evaluating two particular lines of code, after some warm-up

  • default (top) is the default scala console
  • ammonite-current (2nd) is the current master of Ammonite, commit 53edc31
  • ammonite-tty-optim (3rd) is just the first commit
  • ammonite-optim (bottom) is this PR

I couldn't run my benchmark as is with the master of Ammonite (seems the stty runs aren't fine with pasting large blobs of code in the terminal…), so it's a very slightly different way of running the benchmark for that.

Once the CI is green here, I can post the detailed methodogy if that's of interest to anyone…

@alexarchambault

This comment has been minimized.

Collaborator

alexarchambault commented Aug 23, 2018

I have only a ~surface understanding of the stuff I changed… (compiler discarding, and stty tangling) Have a careful look at them, before possibly merging that…

alexarchambault added some commits Aug 23, 2018

Don't refresh compiler upon dynamic classpath change
These basically happen for every cell…
Don't bump Frame version upon import addition
These happen for almost every cell, and don't actually change
the classpath (they're just imports…)
@alexarchambault

This comment has been minimized.

Collaborator

alexarchambault commented Aug 24, 2018

Preliminary benchmark instructions here

@alexarchambault

This comment has been minimized.

Collaborator

alexarchambault commented Aug 28, 2018

I'll probably merge that in the coming days, unless anyone opposes.

About the commented out failing test case, it seems unrelated to the core of this PR… More likely an unearthed corner case in TPrint (it only happens in 2.12, when class wrapping is enabled, the latter not being enabled by default).

The only point I'm not too sure about is whether this can mess with the terminal when one runs commands with % and %%. I guess the terminal state could be applied again right after a command is run. It can always be done in another PR in any case…

alexarchambault added some commits Aug 28, 2018

Fix unnecessary compiler refresh with interp.preConfigureCompiler
Compiler was refreshed after each line of code, after
interp.preConfigureCompiler was called. With this commit, it's only
refreshed once (as was originally intended I guess).
@alexarchambault

This comment has been minimized.

Collaborator

alexarchambault commented Aug 29, 2018

So, merging!

@alexarchambault alexarchambault merged commit 53fcfcd into lihaoyi:master Aug 29, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@alexarchambault alexarchambault deleted the alexarchambault:topic/optim branch Aug 29, 2018

alexarchambault added a commit to almond-sh/almond that referenced this pull request Aug 30, 2018

Switch to latest Ammonite snapshot
Includes lihaoyi/Ammonite#853 in particular,
which makes running several cells in a row feel much faster here

alexarchambault added a commit to almond-sh/almond that referenced this pull request Aug 30, 2018

Switch to latest Ammonite snapshot
Includes lihaoyi/Ammonite#853 in particular,
which makes running several cells in a row feel much faster here

alexarchambault added a commit to almond-sh/almond that referenced this pull request Aug 30, 2018

Switch to latest Ammonite snapshot (#215)
Includes lihaoyi/Ammonite#853 in particular,
which makes running several cells in a row feel much faster here
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment