1505589 - Make Profiler Counters usable within the JavaScript Engine

Status: NEW

(Core :: JavaScript Engine, enhancement, P2)

Description

[Matthew Gaudet (he/him) [:mgaudet]]

Bug 1464509 added some really interesting support for capturing counter values into profiles. There's a lot of really interesting use cases I can imagine for this. 

However, interaction with the profiler from within SpiderMonkey requires some facade to be built, in order to support embeddings that are not Gecko (see js/src/vm/GeckoProfiler.h) 

It appears in the discussion for Bug 1464509 this was discussed, but I don't think the work was completed.

Comment 1

[Randell Jesup [:jesup] (needinfo me)]

Attachment: Make profiler counters usable from js/* source

WIP patch; compiles clean

Comment 2

[Paul Bone [:pbone]]

Comment on attachment 9034480 [details] [diff] [review]
Make profiler counters usable from js/* source

Review of attachment 9034480 [details] [diff] [review]:
-----------------------------------------------------------------

I had a bit of a sticky-beak at these changes.  Hope you don't mind the early feedback.

Thanks for doing this!

::: js/src/vm/GeckoProfiler.h
@@ +132,5 @@
> +  /* functions to support Profiler counters */
> +  void setCounters(profiler_sampled_counter_t add,
> +                   profiler_sampled_counter_t remove);
> +  void addCounter(BaseProfilerCount* aCounter) {
> +    (*addCounterPtr)(aCounter);

What if addCounterPtr is null.  Maybe we should silently not add/remove counters if the profiler has provided nullptr for these functions or maybe there are other reasons it hasn't set them?

::: mfbt/ProfilerCounts.h
@@ +161,5 @@
> +
> +  void Add(int64_t aNumber) { mCounter += aNumber; }
> +
> +  ProfilerAtomicSigned mCounter;
> +};

Can I have a function to reset the counter, not to zero but to a value I calculate?

I hope to count allocations (in bytes) and then reset it to the currently used amount of memory after a GC finishes.

I can add this later if you'd prefer.  It's also not an immediate priority, we don't have a use case for such accurate information yet.

I'd also like something I can increment from JIT-compiled code.  But that's just a way to take the address of the atomic int64 - really simple.  Just FYI in case the internals of this change in the future.

Comment 3

[Randell Jesup [:jesup] (needinfo me)]

Attachment: Make profiler counters usable from js/* source

incorporated pbone's comments.  Added counter->Reset(value)

Comment 4

[Randell Jesup [:jesup] (needinfo me)]

Forwarding to Gerald

Comment 5

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

I won't have time for this, sorry.