-
-
Notifications
You must be signed in to change notification settings - Fork 74
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Timeline Performance boost: Round 2 #65
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We're going to base our code of this
Using event handlers rather than observables to track scrolling If we debounce it, we can potentially call refresh a lot less
Per magento 2 bug: magento/magento2#8084
This will allow us to "cache" the offset rather than searching and calculating every time
MomentJs is really slow. Let's use Date() instead
Not sure why we're doing this so it's going bye bye
Let's not get too fancy and add more methods to the call stack than we have to...
This was slowing things down tremendously. We only want to call it on this page so we insert a block that outputs configuration changes to requirejs to "remap" dom-observer.js to our empty file
Still getting some errors because of this in 2.2.5
It makes it feel sluggish
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Timeline Performance Boost: Round 2
We can now load up-to 35,000 (more on this number below) cron jobs in the timeline, and do so with little to no lag/layout thrashing.
Page load time has decreased significantly (especially when loading large amounts of data), and NO browser cashes! 🎆
(really) fixes issue #53 and works around issue #50
The magic behind this is the new virtualization implementation: VirtualForEach 🔥
VirtualForEach
Like the name suggests, the premise behind this node is to "virtualize" elements (cron job tasks) on the timeline. In other words, if a node can be viewed on the screen (meaning it's not hiding behind any element, or it's within the view port of the window) it will load it into the DOM. Oppositely, if an element is NOT visible, it won't load it into the DOM; thus making this implementation much more memory efficient. VirtualForEach knows exactly where a cron task will be on the timeline by "preCalculating" it's X and Y offsets before it's loaded on the DOM.
We're basically "lazy loading" elements to the DOM, so we don't get caught up with painting/layout calculations that would slow us down.
As the user browses around the timeline, VirtualForEach will keep track of all elements on the timeline; asynchronously, "materializing" and "de-materializing" cron job tasks as needed.
What Exactly Is It?
VirtualForEach is a knockoutJs custom binding that is used by the Cron Job Manager's Timeline that allows us to load unlimited amounts of data into it, without any problems.
This will replace the fastForEach custom binding, by injecting a new "node" to be rendered by Magento's custom rendering engine.
In it's current state, it's not reusable, but maybe in the future it can be.
Why Exactly 35,000 Crons?
This number was picked because it's before the number where Magento's uiComponents cannot handle any more data coming in from the server. See the below comment pertaining to this bug: magento/magento2#8084 (comment)
Just for fun, I patched up the bug using the "LIBXML_PARSEHUGE" fix, and was able to load over 200k crons on the table very quickly.
Hopefully Magento will fix this bug...
Other Notable Updates
Improved loader UX
"Total records loaded" noted on top of the timeline
Demo