davidjbradshaw / iframe-resizer

Keep iFrames sized to their content.
https://iframe-resizer.com
Other
6.63k stars 978 forks source link

Time overhead when adding many elements under a same HTML element #1191

Closed jbousque closed 2 months ago

jbousque commented 8 months ago

Describe the bug When adding, for example, 1000 <div> elements under a same bootstrap .row element, in a child iframe page, the overhead before the page gets rendered can grow exponentially. Adding 1000 items inside 1 row, autoResize=true : addImageLoadListners measured 1min30 in my context. Adding 6 items inside ~166 rows each: addImageLoadListners measured ~2,5 seconds in my context. Adding 1000 items inside 1 row, autoResize=false : addImageLoadListners no overhead obviously

To Reproduce Steps to reproduce the behavior:

  1. With vuejs (here v0.11) and bootstrap for example
  2. Create structure with iframeresizer (parent + iframe pages) with autoResize=true
  3. Create a basic list structure in child page:
    <div class="row"><div class="item" v-repeat="items">[...stuff...]</div></div>
  4. Change nb of items in vue (to 1000 for example)

In my case [...stuff...] can get complex, I suppose the overall time spent really depends on this. Replace vuejs directives by any javascript that would insert the 1000 items 1 by 1 inside the bootstrap .row.

Expected behavior It probably should not take so long.

For now I will add the overhead to split rows where needed, as it mostly workarounds the problem, this issue is to let you know about this finding.

Desktop

Additional context Instrumenting shows that time is spent in iframeResizer.contentWindow.js, inside setupBodyMutationObserver(), here (line 554):

        Array.prototype.forEach.call(
          mutation.target.querySelectorAll('img'),
          addImageLoadListener
        );

This is repeatedly called on the same element, while it is growing 1 by 1 due to how VueJS adds children with the v-repeat. Almost all the time seems to be spent on the querySelectorAll.

Only solution I can think of (except splitting rows appropriately of course) would be to allow deactivating mutationObserver before changing the list, then reactivating it after, dynamically, which currently is not possible if I'm not wrong. Also in this case we would want it to run after re-activation, so iframe height gets properly computed once the whole list is displayed (and not: after each element is added). Alternatively it could be enough to have an option to deactivate image load listeners altogether, or to have a way to exclude their creation based on some tag inside html markup ? It's a bit convoluted.

Note: not tested on iframeResizer v4, I use v3 for compatibility.

davidjbradshaw commented 8 months ago

I think the issue is adding elements one at a time to the page, you really need to add them together as a single block.

davidjbradshaw commented 8 months ago

Thinking a little more, it might help if that loop were denounced to next animation frame by using window.requestAnimationFrame(). By doing it this way all of your inserts should hopefully run before the paint function in the browser render is run. It fixed a similar issue a few years ago and describe in a talk about redux performance I made in 2019 which you can find a link to at https://github.com/davidjbradshaw/redux-performance-talk

 if (!rafId) {
   rafId = window.requestAnimationFrame(
      () => {
           rafId = null
           setupBodyMutationObserver()
       }
   )
}

My health isn’t great at the moment and don’t have much energy to play with this, but it would be great if you give this a go and make a PR if it works.

davidjbradshaw commented 5 months ago

Finally have time to look at this, but I will need a simple case added to the examples to be able to test it.

davidjbradshaw commented 4 months ago

I'm working on v5 #1212 which changes how mutationObserver is used. Would be interested to know if it helps with your issue.