Run OhMyREPL keybindings in a fixed world age #321
Conversation
|
This PR indeed removes all (noticable) latency even when running with JuliaSyntax 0.3.4 (before Kristoffer's recent type-stability improvements to JuliaSyntax) and importing ProgressLogging. When using the latest JuliaSyntax, loading ProgressLogging still leads to 277 invalidations before your PR, and 253 with your PR. In either case, latency is not noticable even when the invalidated code is in the REPL - maybe I just can't figure out how to trigger the invalidated code to recompile. Still, from a latency viewport this is a minor win. If there a no/few downsides, it looks good. If there are any downsides to running in a fixed world age, perhaps a better solution might be to make REPL.jl more type stable. |
Right. This doesn't prevent invalidations in the latest world. But running the key bindings in an older world avoids the compiler having to do work to deal with them. So latency should be much improved. |
c42f
force-pushed
the
c42f/fixed-world-age
branch
2 times, most recently
from
6ece42b
to
b360ca4
Compare
Codecov ReportPatch coverage:
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. Additional details and impacted files@@ Coverage Diff @@
## master #321 +/- ##
==========================================
- Coverage 56.88% 51.67% -5.22%
==========================================
Files 15 13 -2
Lines 1111 1134 +23
==========================================
- Hits 632 586 -46
- Misses 479 548 +69
☔ View full report in Codecov by Sentry. |
|
Good to go? |
c42f
force-pushed
the
c42f/fixed-world-age
branch
from
b360ca4
to
a23a17d
Compare
|
I've updated this to also fix the world age of the pirated |
|
Oh I didn't see your comment. Yes, I think it's good to go now :-) |
This should avoid most invalidations caused by loading other packages. Also remove the NEW_KEYBINDINGS global as it's hard to follow where this is added to. Rearrange all overrides so the implementation code can be precompiled, with the only thing which is `eval`d at runtime being the method pirating itself. Include global mutable "hook" binding for `LineEdit.refresh_line` to allow us to fix the world age of the implementation.
c42f
force-pushed
the
c42f/fixed-world-age
branch
from
a23a17d
to
b76197c
Compare
|
Ugh this has gotten stale/conflicted. I've fixed the conflicts and pushed again. It would be nice to merge this? it contains improvements to precompilation, along with the |
|
Windows nightly test failures are probably due to JuliaDebug/TerminalRegressionTests.jl#14 ? |
Hopefully this should avoid most invalidations caused by loading other packages as discussed in JuliaLang/JuliaSyntax.jl#252.
Also refactor/replace the NEW_KEYBINDINGS global as it's hard to follow where this is added to, and surface a bunch of other uses of global variables like
Base.active_replup to the top level init.I'm not sure whether there's some additional wrapping which is needed around the pirating in
refresh_lines.jl, etc?Seems to fix #316 ?
@jakobnissen You've used OhMyREPL a lot more than me, perhaps you could give this a test drive? I'm not entirely sure of all the ways I should be trying to test this.