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

Make the jumplist infinite #6677

Closed
wants to merge 1 commit into from

Conversation

Philipp-M
Copy link
Contributor

@Philipp-M Philipp-M commented Apr 9, 2023

As I was annoyed a little bit about the short jump list at times, I made it infinite.

Maybe it makes sense to make this configurable non the less. But I think since one jump is only 64 bytes (most of the time) I don't see a reason why it shouldn't be infinite by default.

@the-mikedavis
Copy link
Member

The limit was intentional and it's for limiting the cost of updating the selections rather than space savings: #4750

@Philipp-M
Copy link
Contributor Author

Ah makes sense, ok, maybe we should do it lazily at some time too? I'm closing this in the meantime.

@Philipp-M Philipp-M closed this Apr 9, 2023
@the-mikedavis
Copy link
Member

We do apply some changes lazily: the focused view is always updated eagerly but other views are updated just before changing focus (<C-w>w and others). Applying it lazily is a little bit of a double-edged sword though - when we focus a new view and apply all the changes, we might have to do a lot of work all at once which could cause a pause.

@Philipp-M
Copy link
Contributor Author

Good to know, thanks for the explanation.

How I think it would makes sense to update jumps lazily without having performance problems with infinite jump lists, is to only apply changes to each jump when the jump (i.e. the selection(s) of the jump and actually all other revision based state) is actually required.

Something like this:

Instead of using an apply method for updating all jumps in a view at once, manage a revision id for each jump, and as soon as the selection of a jump is required, it is updated with all the transactions in the history of that document since that revision (in either direction, e.g. if undo was pressed revision may be newer in the jump).

Actually I started implementing this, but I gave up after some time, as e.g. the mutability requirement for a jump that makes things a little bit more difficult (I'm not sure how safe interior mutability/runtime borrow checking is and didn't want to deep dive into that right now). Maybe we don't need to cache/update the selection/revision id, and just compute it on the fly, needs more research...

I'm not sure if it's worth the amount of work involved here (for now), and I think this should probably done in different (deeper) abstraction layer etc. (some kind of a selection/revision based manager, that lazily reacts to the current version of the document, when the selection is needed).

The more I'm reading into CRDTs the more I think it could actually be of help here and various other things (in core), but this is a whole different story...

I'm keeping the infinite jumps in my personal fork for now, and maybe revisit this at a later time.

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