Skip to content

Conversation

harrishancock
Copy link
Collaborator

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.

@anonrig
Copy link
Member

anonrig commented Sep 16, 2025

There is a conflict due to the revert of performance changes. The new PR that reverts the revert of revert is #5103

@harrishancock
Copy link
Collaborator Author

I'll wait for the dust to settle. :)

@anonrig
Copy link
Member

anonrig commented Sep 16, 2025

I'll wait for the dust to settle. :)

Appreciate it!

@harrishancock harrishancock force-pushed the harris/EW-9466-perf-event-mark branch from a73ab30 to 7efe07f Compare September 16, 2025 17:03
@harrishancock harrishancock requested review from a team as code owners September 16, 2025 17:03
@harrishancock harrishancock changed the base branch from main to yagiz/revert-revert-revert-performance September 16, 2025 17:03
@harrishancock harrishancock force-pushed the harris/EW-9466-perf-event-mark branch from 7efe07f to 8e86810 Compare September 16, 2025 17:07
@jasnell
Copy link
Collaborator

jasnell commented Sep 16, 2025

Once the revert is all settled out, you might consider also adding support for this in the performance.mark(...) API

@anonrig anonrig force-pushed the yagiz/revert-revert-revert-performance branch 7 times, most recently from 88b8391 to c1aff5f Compare September 18, 2025 19:01
Base automatically changed from yagiz/revert-revert-revert-performance to main September 18, 2025 19:40
@anonrig
Copy link
Member

anonrig commented Sep 19, 2025

Hi @harrishancock, can you rebase?

@harrishancock harrishancock force-pushed the harris/EW-9466-perf-event-mark branch from 8e86810 to 6bd308d Compare September 19, 2025 13:19
@harrishancock
Copy link
Collaborator Author

Done. I added instrumentation to .mark() and .measure() as well.

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.

@harrishancock harrishancock force-pushed the harris/EW-9466-perf-event-mark branch 3 times, most recently from c024360 to 0ff0dae Compare September 24, 2025 10:59
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.
@harrishancock harrishancock force-pushed the harris/EW-9466-perf-event-mark branch from 0ff0dae to e150a22 Compare September 24, 2025 11:00
@harrishancock harrishancock merged commit ccfa6bf into main Sep 24, 2025
21 checks passed
@harrishancock harrishancock deleted the harris/EW-9466-perf-event-mark branch September 24, 2025 13:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants