Skip to content

Conversation

@jackwilsdon
Copy link

I'm not sure why we do this but it causes large amounts of lag with binary files - removing it doesn't seem to have any effect I can see.

Removing this mitigates zyedidia/micro#1209.

@jackwilsdon
Copy link
Author

jackwilsdon commented Oct 2, 2018

It looks like calling ReOpen is actually an accidental holdover from a change which increased performance with large files, but it was only useful when we opened a new file like so (see 8a53791);

CurView():VSplitIndex(NewBuffer("", filename), 1)
CurView():ReOpen()

This was changed to actually open the file in a2d0408 (yet again to increase performance), but the ReOpen was left behind;

CurView():VSplitIndex(NewBufferFromFile(scanlist[y].abspath), 1)
CurView():ReOpen()

So I don't think there's any real reason to keep the ReOpen call around anymore, as it's just double-opening the file and causing micro to perform a diff between the versions.

@sum01
Copy link
Collaborator

sum01 commented Oct 3, 2018

I know it used to be used because without it the buffer wouldn't refresh, thus looking like nothing was opened. So does VSplitIndex do it for us now?

I should get around to testing this soon.

@jackwilsdon
Copy link
Author

jackwilsdon commented Oct 3, 2018

Well previously we called NewBuffer directly, which allowed us to create an empty buffer with a specific filename. Calling ReOpen was required then to actually load the file data, as we weren't loading it into the buffer initially.

Now that we use NewBufferFromFile, it already loads the data from disk for us and calling ReOpen just loads it a second time.

@sum01 sum01 merged commit 24c404c into NicolaiSoeborg:master Oct 3, 2018
sum01 added a commit that referenced this pull request Oct 3, 2018
Performance improvement

Thanks to @jackwilsdon, ref PR #37
@jackwilsdon jackwilsdon deleted the no-extra-reopen branch October 3, 2018 22:00
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