Problem/Motivation
The attach behavior for the horizontal tabs might get really slow depending on how much content will be under each horizontal tab. The problem is that the attach behavior uses the jquery .wrap() function which relies on the .appendChild() function. As an example in my case the horizontal tabs processing takes 2105ms! You might look at the following screenshot from the java script profiler.
Proposed resolution
1. Instead of modifying the DOM to wrap the content for the horizontal tabs in a outer div, just place the outer div in the twig template and there is no need for a DOM modification anymore.
2. Instead of modifying the DOM to insert the content for the tabs column, just place it in the twig template and there is no need for a DOM modification anymore.
Measurements for the total execution duration of the function Drupal.attachBehaviors:
The current approach:
10 subsequent reloads result in an average total execution time of 2189,8ms.
With the proposed resolution:
10 subsequent reloads result in an average total execution time of 1499,8ms.
Result:
An amazing execution speed up of 690ms!
Remaining tasks
Review.
User interface changes
none
API changes
none
Data model changes
none
Comment | File | Size | Author |
---|---|---|---|
#15 | 2760835-15.patch | 6.17 KB | kfritsche |
#13 | 2760835.patch | 6.05 KB | DerryC |
#9 | 2760835-9.patch | 6.06 KB | hchonov |
#7 | 2760835-7.patch | 6.37 KB | hchonov |
#5 | interdiff-2760835-4-5.txt | 575 bytes | aby v a |
Comments
Comment #2
hchonovComment #3
hchonovRethinking my previous proposal : we do not need a new implementation, but we still can get rid of .wrap and .before used in the attach behavior by placing this div elements directly into the twig template. By doing so and expecting them to be there I've achieved a really good performance optimization when having a really a lot of content.
Measurements for the total execution duration of the function Drupal.attachBehaviors:
Without the patch:
10 subsequent reloads result in an average total execution time of 2189,8ms.
With the patch applied:
10 subsequent reloads result in an average total execution time of 1499,8ms.
Result:
An amazing execution speed up of 690ms!
Comment #4
hchonovActually we could limit the search for the new elements directly to the first children.
Comment #5
aby v a CreditAttribution: aby v a at Zyxware Technologies commentedHi,
I have changed the comment with a full stop. Kindly check the patch.
Comment #6
nils.destoop CreditAttribution: nils.destoop as a volunteer and at iO commentedI'll check the patch. Did you also profile the vertical tabs? Horizontal tabs is a copy of vertical tabs. So i think they will have the same issue (unless they changed code since fieldgroup started)
Comment #7
hchonovNo, I haven't checked what is the current state of vertical tabs.
Re-rolling the patch as it could not be applied anymore.
Comment #8
cspitzlayAny chance to get this optimization into 1.0 (once it is rerolled)?
Comment #9
hchonovRe-roll...
Comment #11
hchonovSo the only fail is where the whole branch is currently failing so not a problem with my patch.
Comment #12
campbellt_ CreditAttribution: campbellt_ at Lil Engine commentedClosing due to outdated. Please reopen if still an issue.
Comment #13
DerryC CreditAttribution: DerryC commentedRe-roll for 8.x-3.x...
Comment #14
idebr CreditAttribution: idebr at iO commentedComment #15
kfritscheRe-roll.
Comment #16
nils.destoop CreditAttribution: nils.destoop as a volunteer and at iO commentedThx for the patch and re-rolls. I added an extra visually-hidden on the list, as it would display an empty box when javascript is disabled.
I also switched to data attributes for the selectors.
Comment #18
nils.destoop CreditAttribution: nils.destoop as a volunteer and at iO commented