-
Notifications
You must be signed in to change notification settings - Fork 132
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
Editor invalidation, optimizations, and more #433
Draft
MinusGix
wants to merge
21
commits into
lapce:main
Choose a base branch
from
MinusGix:inval
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Last vline could grow very expensive (10ms) on large (200k lines) files, because it was forced to look up every (font_size, line) in the hashmap. (Possibly it was more expensive than 10ms previously, because it had a second-level hashmap rather than an array!) Now it is quite cheap as the new layout cache makes it simple to only do that iteration on the lines actually in view. It does have the issue that it isn't respecting different font sizes for each line but that's fixable.
Register could have large text in its fields which we would then clone everytime we performed an action relating to it.
This was originally only thought to be used for code lens in Lapce, which would shrink the font size of lines, but I don't believe that's implemented. As well, it should be fast enough to just rerender those text layouts. This makes many operations much faster, no longer having to deal with a hashmap or the trait object indirection every loop iter.
This significantly simplifies & optimizes the calculation of the initial visual line, now being mostly only sensitive to how many lines are on screen than the size of the file. As well it lets us get rid of a bunch of side code that isn't needed.
Uses a listener rather than invalidating only the editor doing the edit, for the obvious reason so as to invalidate all editors. (Though I would've liked to avoid using Listener) Also fixes & simplifies the invalidation calculation, adding tests to ensure correctness.
Also does some minor changes to Document
This is useful for Lapce where we have styles/code-actions/etc associated with lines which can be partially invalidated.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This does a variety of optimizations.
Commits:
HashMap<Font Size, HashMap<Line, TextLayout>>
, we store it as (roughly)HashMap<Font Size, [TextLayout]>
.InvalLines
, rather than the recreating the text layouts after every editlast_line
of the Rope on theRopeTextVal
/RopeTextRef
, as this gives some boost due to how often we compute it.Rope
itself, but it'd require a good investment of time to wrap my head around all the implementation details ofRope
. (which is also why some other opts aren't done)HashMap<Font Size, _>
as it complicates a bunch of code having multiple locations for the layouts to come from.Lines
doesn't need to know the font size. This simplifies the code.InvalCount
not need the rope as the majority of locations do not benefit from it, and it would make simply invalidating some lines harder. Still has the version withRope
separate asInvalCountR
cache_rev
fromDocument
(and the impl supplied,TextDocument
), as well as from Visual Lines, as we no longer need an entire file cache id due to line-specific invalidation. (All the lines can still be cleared by calling a specific function onLines
, possibly should be exposed fromEditor
if it isn't already)Layouts
cache a genericLineRenderCache
which can be used in Lapce for various data associated with lines that needs to be invalidatedThere's still a few bugs I need to determine, and may try other alterations.