search

ractivejs / ractive

Next-generation DOM manipulation
http://ractive.js.org
MIT License
5.94k stars 396 forks source link

Some bug with `@.link` #3232

Closed dagnelies closed 6 years ago

dagnelies commented 6 years ago

Ok, I'm doing very fancy things lately that cannot fit in a couple of lines.

Currently, I'm facing an issue with a @.link(foo, bar) having each consecutive call taking about twice as much time as before, including hundreds of GC MBs. I cannot isolate it right now, I can just see that the rebind and markedAll seem to go through tons of objects that pile up. The data object itself is tiny.

I suspect there is something wrong with the building up or traversal of LinkModel.children, but I con only do guesswork.

As a guesswork hint, it appeared at the same time as I introduced a "neighboring" reference {{linked.object}} and somewhere else {{#each linked.object.children}}{{../../settings.detail}}{{/each}} ...although I've no idea it might be the cause or a coincidence.

dagnelies commented 6 years ago

If I unlink the previous selection beforehand, it appears to be fine. Perhaps we could do that preventively.

fskreuz commented 6 years ago

Might be helpful to post a CPU profile/flame graph/stack trace/memory snapshot. Not very familiar with the profiling features of Chrome Dev tools, especially memory debugging, but I've seen @evs-chris and several others fix leaks using the mentioned info.

evs-chris commented 6 years ago

Is there something you can share publicly or privately that I can take a look at? It sounds like a fairly serious issue, but a perusal of the relevant code didn't surface any obvious issues.

dagnelies commented 6 years ago

Hi,

sorry for the delay, I was gone for a while. Here is the performance profiler output, I think you can import it in chrome.

Profile-20180502T114519.zip

evs-chris commented 6 years ago

I think I have an idea as to where this is getting chewy. I'll probably need an example I can step through to confirm, but it looks like some of the bookkeeping for named bindings is part of the problem. There are already some issues with named bindings and the actual names that come out, so both of these can probably be addressed at the same time.

It also looks like links that have reference expressions built on top of them may be doing some of the same work twice, but I haven't poked at that side very hard yet.

dagnelies commented 6 years ago

ractive-builder.zip

Here is the experiement thingy. The bug can easely be reproduced by dragging a few components from the left panel in the middle and then repetitively selecting one "widget" then another. At first, it's not noticeable but each click will roughly double the event processing time. From a millisecond to long enough to freeze the browser.

evs-chris commented 6 years ago

Found it! There was a bit of a logic error with rebinding wherein a reference expression proxy would reregister itself with its upstream deps even if they didn't change, which causes an exponential increase in deps for each rebind. This should be fixed in edge and v0.10-dev as soon as travis is done doing what travis does. Thanks for the report!

dagnelies commented 6 years ago

welcome, thanks to you