-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Speed up snapshotting of many new dom nodes #903
Conversation
By avoiding reflow we shave about 15-25% off our snapshotting time
Makes it easier to profile
Very interesting! Going to learn the details. |
Because they only do the .lenght check once. In this case I don't think we'll see much performance gains if any
@Yuyz0112 I've finished tweaking this branch. Feel free to review it. Happy to answer any questions if you have them |
🤩 |
// Scroll | ||
if (!newlyAddedElement) { | ||
// `scrollTop` and `scrollLeft` are expensive calls because they trigger reflow. | ||
// Since `scrollTop` & `scrollLeft` are always 0 when an element is added to the DOM. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
interesting approach 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering whether we could replace newlyAddedElement
and skipChild
with a single fromMutation
argument as they are both true/false in the same places; I feel like that would suffice and be clearer, but let me know thoughts before I pursue that PR!
Co-authored-by: yz-yu <[email protected]>
LGTM, we can merge this after conflicts are resolved. |
Bumps [minimist](https://github.com/substack/minimist) from 1.2.5 to 1.2.6. - [Release notes](https://github.com/substack/minimist/releases) - [Commits](https://github.com/substack/minimist/compare/1.2.5...1.2.6) --- updated-dependencies: - dependency-name: minimist dependency-type: indirect ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@Yuyz0112 fixed the merge conflicts. Feel free to merge this branch into master |
}; | ||
|
||
it('should serialize scroll positions', () => { | ||
const el = render(`<div stylel='overflow: auto; width: 1px; height: 1px;'> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
was this a typo? stylel
By avoiding reflow we shave about 15-25% off our snapshotting time.
By doing some other tweaks we can bring our savings up to about 40-50%.
Before:
After: