timdown / rangy

A cross-browser JavaScript range and selection library.
MIT License
2.24k stars 368 forks source link

Rangy is very slow in some circumstances #287

Open DeadMG opened 9 years ago

DeadMG commented 9 years ago

I have a DOM structure where each individual word in the DOM is tagged with a tracking span (so each space is it's own individual text node). Once the number of words becomes even moderate, like 100, the TextRange functionality of Rangy is particularly slow- hundreds of milliseconds to do things like convert to character ranges.

timdown commented 9 years ago

Yes. It cannot be denied. It does a lot of DOM calls, some of which are expensive.

If you're not changing the DOM at all, you could try wrapping a bunch of TextRange calls inside rangy.noMutation as follows:

rangy.noMutation(function() {
    // Put your TextRange-related calls in here
});

I'd be interested to know if that helps. In theory it might do because it caches computed styles and other things. However, this is not a documented feature; what to do about it is on my list of things to think about, although probably not until after 1.3 is released.

DeadMG commented 9 years ago

noMutation had absolutely no effect. In this case we're talking about the context of a single call to Rangy, so it's impossible for the DOM to be mutated unless you mutate it in your implementation.

The problem seems to be that getCharacter() recurses into itself nearly indefinitely and hardly ever makes progress, so the DOM calls just keep stacking up and up.

timdown commented 9 years ago

I have seen that kind of thing in some tests but thought I'd fixed it. Do you have an example document?

DeadMG commented 9 years ago

Let me see if I can make a reproduction for you.

DeadMG commented 9 years ago

Found the issue.

We render our scripts into the body of the document, and then the range work is being done in a temporary div at the bottom. When Rangy tries to find the previous visible character, that would require iterating through 577 script tags to find the actual content of the document to find the previous visible character. They're bundled in production but not for development. This also explains why I had problems reproducing- because obviously the first thing I did to make the example minimal was delete the script tags and add the relevant scripts to the bottom.

I succeeded in working around this issue by rendering the script tags inside a display:none div.

I guess that in general, it surprises me that to operate on an element, Rangy even looks at the surrounding DOM. I would have expected that a div with given contents always behaves the same no matter where it is.

timdown commented 9 years ago

It looks ahead/behind to decide whether the start or end of a block element or a <br> element should imply a line break, which can only be inferred from context. However, it sounds as though it is looking ahead/behind a little too eagerly in some cases.

DeadMG commented 9 years ago

I think also what I would really appreciate is the ability to filter some DOM nodes from these expensive DOM calls. In the original case that caused me to open this ticket, none of those spans could possibly have invisible content or newlines in them, for example, so a lot of time would have been wasted looking at them.

apantsiop commented 9 years ago

I have also noticed that when using range.selectCharacters() as you can see in this fiddle:

https://jsfiddle.net/axdu5898/1/

It gets slower as nesting of elements becomes deeper.