-
Notifications
You must be signed in to change notification settings - Fork 441
EW-9466 EW-9596 Add perf event marking to performance.now() #5097
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
There is a conflict due to the revert of performance changes. The new PR that reverts the revert of revert is #5103 |
I'll wait for the dust to settle. :) |
Appreciate it! |
a73ab30
to
7efe07f
Compare
7efe07f
to
8e86810
Compare
Once the revert is all settled out, you might consider also adding support for this in the |
88b8391
to
c1aff5f
Compare
Hi @harrishancock, can you rebase? |
8e86810
to
6bd308d
Compare
Done. I added instrumentation to I suppose it would make sense to move those two events to the PerformanceMark and PerformanceMeasure class constructors, but I didn't want to think too hard about the change right now. |
c024360
to
0ff0dae
Compare
This commit contains two changes: - Adds an IsolateLimitEnforcer::markPerfEvent(name) function whose default implementation is a no-op. Its internal implementation injects perf counter data into our perf_event observability system. - Calls the function from performance.now(). This allows us to see what our perf_event counters are at the moment in time when JS calls that API.
I added perf event marks to Performance.mark() and Performance.measure(). It'd probably make a lot of sense to pass the mark name as the perf event's name, but our system currently requires the names to be static strings, so I just left them as "performance_now" and "performance_measure". Note that I changed calls to now(js) to the underlying implementation, dateNow(), to avoid spuriously injecting extra "performance_now" events.
0ff0dae
to
e150a22
Compare
This commit contains two changes: