Skip to content
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
Merged

Optims #853

merged 6 commits into from
Aug 29, 2018

Conversation

alexarchambault
Copy link
Collaborator

@alexarchambault 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
Copy link
Collaborator Author

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
Copy link
Collaborator Author

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…

These basically happen for every cell…
These happen for almost every cell, and don't actually change
the classpath (they're just imports…)
@alexarchambault
Copy link
Collaborator Author

Preliminary benchmark instructions here

@alexarchambault
Copy link
Collaborator Author

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…

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
Copy link
Collaborator Author

So, merging!

@alexarchambault alexarchambault merged commit 53fcfcd into com-lihaoyi:master Aug 29, 2018
@alexarchambault alexarchambault deleted the topic/optim branch August 29, 2018 15:15
alexarchambault added a commit to almond-sh/almond that referenced this pull request Aug 30, 2018
Includes com-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
Includes com-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
Includes com-lihaoyi/Ammonite#853 in particular,
which makes running several cells in a row feel much faster here
alexarchambault added a commit to alexarchambault/Ammonite that referenced this pull request Jan 24, 2019
Have Ctrl-C not send SIGINT to us. Ctrl-C are then handled by terminal
filters, like before (current input line is ignored, new prompt is
printed below, accepting new input).

This runs a command right before and after accepting input, which has
some performance implications… This reverts some gains of
com-lihaoyi#853.
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.

None yet

1 participant