JSOPTION_PCCOUNT should use js::Interpret's interrupts to count opcodes

RESOLVED FIXED in mozilla9

Status

()

Core
JavaScript Engine
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: jimb, Assigned: jimb)

Tracking

unspecified
mozilla9
x86_64
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

6 years ago
Created attachment 553261 [details] [diff] [review]
Use js::Interpret's interrupts to implement JSOPTION_PCCOUNT's bytecode profiling.

At the moment, the DO_OP macro in js::Interpret calls COUNT_OP, which expands to:

JS_BEGIN_MACRO                                     \
    if (pcCounts && !regs.fp()->hasImacropc())     \
        ++pcCounts[regs.pc - script->code];        \
JS_END_MACRO

It doesn't seem ideal to put a branch on the hot path of every opcode dispatch. Instead, when the script has a pcCounts table, the interpreter should use the interruptJumpTable, and the 'interrupt:' path should count the opcodes.
Attachment #553261 - Flags: review?(sphink)
(Assignee)

Comment 1

6 years ago
Created attachment 553268 [details] [diff] [review]
Use js::Interpret's interrupts to implement JSOPTION_PCCOUNT's bytecode profiling.

Improved macro names.
Pulled "pcCount -> interrupts enabled" assertion out into a macro, made it check for both threaded and non-threaded interpreters.
Attachment #553261 - Attachment is obsolete: true
Attachment #553261 - Flags: review?(sphink)
Attachment #553268 - Flags: review?(sphink)
Comment on attachment 553268 [details] [diff] [review]
Use js::Interpret's interrupts to implement JSOPTION_PCCOUNT's bytecode profiling.

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

I really wish I had thought of this approach. It's much, much better than the original, and obsoletes my current patch to add configure flags and variant outputs to avoid the perf impact.

Patch looks great to me.
Attachment #553268 - Flags: review?(sphink) → review+
(Assignee)

Comment 3

6 years ago
(In reply to Steve Fink [:sfink] from comment #2)
> Comment on attachment 553268 [details] [diff] [review]
> Use js::Interpret's interrupts to implement JSOPTION_PCCOUNT's bytecode
> profiling.
> 
> Review of attachment 553268 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I really wish I had thought of this approach. It's much, much better than
> the original, and obsoletes my current patch to add configure flags and
> variant outputs to avoid the perf impact.
> 
> Patch looks great to me.

I'm glad I could make it work!
(Assignee)

Updated

6 years ago
Blocks: 673125
http://hg.mozilla.org/integration/mozilla-inbound/rev/8391d102fd70
Whiteboard: [inbound]
http://hg.mozilla.org/mozilla-central/rev/8391d102fd70
Assignee: general → jimb
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Whiteboard: [inbound]
Target Milestone: --- → mozilla9
You need to log in before you can comment on or make changes to this bug.