Skip to content

Improve show-whitespace performance on large files#2737

Merged
fregante merged 11 commits into
masterfrom
faster-whitespace
Feb 3, 2020
Merged

Improve show-whitespace performance on large files#2737
fregante merged 11 commits into
masterfrom
faster-whitespace

Conversation

@fregante
Copy link
Copy Markdown
Member

@fregante fregante commented Jan 26, 2020

Closes #2732

I rewrote the feature to limit DOM manipulation. The previous one created a new DocumentFragment for each character, whether it was whitespace or not.

Even this version was slow until I cached textNode.textContent; Firefox really doesn't like reading that many times in a loop. Now it's only read once per replacement, which makes the test URL instant. A related bug was opened here but closed as worksforme: https://bugzilla.mozilla.org/show_bug.cgi?id=1330475

I wonder if manipulation could be further improved with Range.surroundContents but the current solution seems fast enough. Edit: tried, 5x slower: 066402a

Current bugs:

  • for some reason it's also trying to replace other characters like = and {
  • alignment seems off

Test

Careful, this URL will freeze your Firefox window if you open it with the master version of Refined GitHub: https://github.com/teropa/to-sting/blob/master/index.js

@fregante fregante marked this pull request as ready for review January 27, 2020 03:54
@fregante fregante requested a review from notlmn as a code owner January 27, 2020 03:54
@fregante
Copy link
Copy Markdown
Member Author

On my computer, the long https://github.com/microsoft/TypeScript/blob/master/lib/lib.dom.d.ts file performs terribly, with or without Refined GitHub. Now that I dropped the await setTimeout I should make sure that performance on the file isn't impacted.

@notlmn
Copy link
Copy Markdown
Contributor

notlmn commented Jan 27, 2020

Will take a look at this once I'm back at a keyboard!

@fregante
Copy link
Copy Markdown
Member Author

fregante commented Jan 27, 2020

I tested lib.dom.d.ts and it still took between 5s and 25s to complete. I initially added a timer (f6f9f8b) but I think we can skip the work altogether: only parse visible code (7cccc48)

This will make the whitespace appear a couple of frames later but I think it's worth it.

I thought that observing thousands of elements would slow the browser down but I don't see any difference on that page.

@refined-github refined-github deleted a comment from chidinmaik Jan 29, 2020
@refined-github refined-github deleted a comment from bonbon-713 Jan 29, 2020
@fregante
Copy link
Copy Markdown
Member Author

@notlmn let me know today if you want to look at this because I’d like to merge it and publish a new version with this

@fregante fregante changed the title Dramatically faster show-whitespace Improve show-whitespace performance on large files Feb 3, 2020
@fregante fregante merged commit a367b56 into master Feb 3, 2020
@fregante fregante deleted the faster-whitespace branch February 3, 2020 07:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

firefox Related to Firefox only

Development

Successfully merging this pull request may close these issues.

High CPU and RAM usage on large files (due to show-whitespace)

3 participants