Element-level signals for lifecycle events#7205
Conversation
cramforce
left a comment
There was a problem hiding this comment.
Could some parts of the performance service be rewritten in terms of this?
I am pretty worried about the extra memory usage of this. It might be worth limiting to a single array per object where each signal gets 6 (would need to add the name) slots in the array. This would avoid any object allocation per signal that never get a promise created.
src/custom-element.js
Outdated
| */ | ||
| signal(name, opt_time) { | ||
| let signal = this.signals_[name]; | ||
| if (!signal) { |
There was a problem hiding this comment.
Assign all fields to undefined to avoid change to object shape.
There was a problem hiding this comment.
Changed to separate map.
27f00bd to
e1a3b5b
Compare
|
@cramforce Reimplemented via two maps: one for result and one for promise. PTAL. |
|
@cramforce RE:performance. Not yet, but some aspects in Resources will eventually be made simpler that performance relies on. However, other elements, such as |
* Element-level signals for lifecycle events * fixing tests
* Element-level signals for lifecycle events * fixing tests
We have more and more use cases that need major lifecycle signals produced by standard AMP element and extensions. We can implement each separately, but there are some issues:
This PR:
whenSignal,signalandrejectSignal.signalis the only "always use" method and thus it fills it only minimal parts of the struct and does minimal work. The rest is done lazily.whenBuiltis an example of API-level usage. Some lifecycle signals can be easily exposed via special API. This is currently in work for another PR. (/cc @mkhatib Wait for amp-selector to build before submitting #7190)signalDeltais an example of how many metrics can be calculated between two signals. This will be (already is) very common in many analytics needs.