♻️ peformance-impl: simplify tick()#26567
Conversation
| } | ||
| // Mark the event on the browser timeline, but only if there was | ||
| // no delta (in which case it would not make sense). | ||
| if (arguments.length == 1) { |
There was a problem hiding this comment.
Why check arguments.length == 1 as opposed to for a null/undefined opt_delta?
| // variables. | ||
| const storedVal = Math.round( | ||
| opt_delta != null ? Math.max(opt_delta, 0) : value - this.initTime_ | ||
| ); |
There was a problem hiding this comment.
I actually find this less readable. I think there's a ToTT for it somewhere but I also found this: https://refactoring.com/catalog/reduceScopeOfVariable.html
There was a problem hiding this comment.
You prefer the opt_delta condition repeated 3 times?
There was a problem hiding this comment.
Speaking of this line, I have a question. Why in the absolute case (relative to epoch) do we set storedVal to the delta?
There was a problem hiding this comment.
answer: its used in the url-rewriter, and non-viewer use cases care more about relative than absolute times (this is still fishy, why do we ever send absolute times).
tick()'s branched logic in a single place near the top, instead of being spread throughout.valueordelta, instead of both.