Add a compilation ID to code generation

ASSIGNED
Assigned to

Status

()

Core
JavaScript Engine: JIT
P5
normal
ASSIGNED
7 months ago
5 months ago

People

(Reporter: sfink, Assigned: sfink)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

7 months ago
For debugging, I thought it would be nice to have a unique compilation ID to distinguish the different compilations.
(Assignee)

Comment 1

7 months ago
Created attachment 8891563 [details] [diff] [review]
Add a compilation ID

I have mixed feelings about this patch. First, I doubt I'm doing it the right way; I don't fully understand the class hierarchy, and I didn't really think about the JIT's thread setup. Second, it's a bunch of fiddly changes and I strongly suspect that I don't need it for what I've been using it for. I just need to parse the spew a little more intelligently. But I'm uploading this in case you think it's useful.
Attachment #8891563 - Flags: review?(nicolas.b.pierron)
Comment on attachment 8891563 [details] [diff] [review]
Add a compilation ID

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

::: js/src/jit/MacroAssembler.cpp
@@ +37,5 @@
>  using JS::ToInt32;
>  
>  using mozilla::CheckedUint32;
>  
> +mozilla::Atomic<int, mozilla::ReleaseAcquire> MacroAssembler::CompilationCounter(0);

follow-up: Wrap all these compilations id under #ifdef JS_JITSPEW.

The reason I am asking for that is that we have the RecompileInfo which is used internally to identify one Ion compilation and I would prefer to avoid having 2 counters which are almost identical.  It would not help you in this case as it does not cover WASM, inline caches, nor trampolines.

::: js/src/jit/MacroAssembler.h
@@ +322,5 @@
>      NonAssertingLabel failureLabel_;
>  
> +    static mozilla::Atomic<int, mozilla::ReleaseAcquire> CompilationCounter;
> +
> +    void initWithAllocator() {

follow-up: Name this function "init". Add a TempAllocator* argument, and move the moveResolver_.setAllocator(…) in it.
Attachment #8891563 - Flags: review?(nicolas.b.pierron) → review+
Priority: -- → P5
You need to log in before you can comment on or make changes to this bug.