-
Notifications
You must be signed in to change notification settings - Fork 6.8k
perf(ripple): optimize event registration #18633
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
Conversation
Improve scalabiliity of Angular Material components. When used in an app with a large and heavy UI with large numbers of material components (for example in table cells), they can quickly become prohibitively expensive due to their heavy DOM structure and lots of up-front work. Each component with a ripple ends up registering 6 event listeners. This PR makes 4 of them registered on-demand. Note, that this is really a stop-gap solution to help improve performance in our application. A proper implementation should utilize a single document-level service that only registers one set of these event listeners, and uses event delegation to apply ripple effects.
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.
@devversion PTAL
this._registerEvents(pointerDownEvents); | ||
} | ||
|
||
/** Handles all registered events. */ |
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.
Could you add the @docs-private
annotation to this JsDoc block? That will prevent it from appearing in our generated API docs.
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.
Done.
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.
LGTM
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.
Looks great to me. Thanks!
}); | ||
|
||
if (this._pointerUpEventsRegistered) { |
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.
the if statements is really needed here ?
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.
It avoids unnecessary iteration and removeEventListener() calls.
* perf(ripple): optimize event registration Improve scalabiliity of Angular Material components. When used in an app with a large and heavy UI with large numbers of material components (for example in table cells), they can quickly become prohibitively expensive due to their heavy DOM structure and lots of up-front work. Each component with a ripple ends up registering 6 event listeners. This PR makes 4 of them registered on-demand. Note, that this is really a stop-gap solution to help improve performance in our application. A proper implementation should utilize a single document-level service that only registers one set of these event listeners, and uses event delegation to apply ripple effects. * Update core.d.ts * Add the @docs-private annotation.
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Improve scalability of Angular Material components.
When used in an app with a large and heavy UI with large numbers of material components, they can quickly become prohibitively expensive due to their heavy DOM structure and lots of up-front work.
Registering event listeners is slow, and gets progressively slower as the DOM tree grows and gets more complex. Each component with a ripple ends up registering 6 event listeners. Consider a 100-row table with 6 buttons per row. That's 3600 event listeners that need to be registered during rendering. This PR makes 2400 of them registered on-demand.
Note, that this is really a stop-gap solution to help improve performance in our application. A proper implementation should utilize a single document-level service that only registers one set of these event listeners, and uses event delegation to apply ripple effects.