Closed Bug 1026905 Opened 6 years ago Closed 6 years ago

IonMonkey: remove the instruction counters from the script counters.

Categories

(Core :: JavaScript Engine: JIT, defect)

ARM
All
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla33
Tracking Status
firefox31 --- fixed
firefox32 --- fixed
firefox33 --- fixed
b2g-v2.0 --- fixed
b2g-v2.1 --- fixed

People

(Reporter: dougc, Assigned: dougc)

References

Details

(Whiteboard: [qa-])

Attachments

(1 file)

The script counter currently counts instruction bytes and spill bytes, and it does so using the size() method to obtain the current buffer position.

The size() method is only accurate on the ARM when pending pools are flushed. Using the size() method with a pending pool to flush is most likely a programming error so I would like to assert that the pools are flushed in the size() method.

It would be possible to use the buffer offset, excluding the pools, but this may not be available in future with an elastic assembler buffer.

This issue has been discussed before and the feedback received at the time was that this code can just be removed so I would like to do so and uplift this to beta (31) in support of the asm buffer work.
Attachment #8441875 - Flags: review?(bhackett1024) → review+
Blocks: 1026919
The try run had some failures on Windows debug build tests but I see the same failures on other close runs and I do not think they are related to this bug.
Keywords: checkin-needed
Comment on attachment 8441875 [details] [diff] [review]
Remove the instruction counters from the script counters.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Feature was added in bug 814489.

User impact if declined: None, but this code is not accurate for the ARM backend and if this were not just used to generate some statistics then it would likely indicate a programming error so an assertion to catch such usage is being added in pending patches. Would like to get this uplifted to ESR 31 to avoid blocking other patches.

Testing completed (on m-c, etc.): Locally, and a try run.

Risk to taking this patch (and alternatives if risky): low.

String or IDL/UUID changes made by this patch: n/a
Attachment #8441875 - Flags: approval-mozilla-beta?
Attachment #8441875 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/d504b084db82
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
Attachment #8441875 - Flags: approval-mozilla-beta?
Attachment #8441875 - Flags: approval-mozilla-beta+
Attachment #8441875 - Flags: approval-mozilla-aurora?
Attachment #8441875 - Flags: approval-mozilla-aurora+
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.