IonMonkey always bails for string[string] GETELEM

RESOLVED FIXED in mozilla28

Status

()

Core
JavaScript Engine: JIT
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: jandem, Assigned: jandem)

Tracking

(Blocks: 2 bugs)

unspecified
mozilla28
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

4 years ago
The jQuery event benchmark in bug 928318 has a "trigger" function that's bailing out from Ion to Baseline every time it's called:

event = event[ jQuery.expando ] ?
	event :
	new jQuery.Event( type, typeof event === "object" && event );

The problem is that |event| is a string, and we try to convert jQuery.expando (also a string) to int32, but this will always bail because string to int32 is complicated. Instead, we should only try to optimize string[x] if x is int32.
(Assignee)

Comment 1

4 years ago
Created attachment 824675 [details] [diff] [review]
Patch

This also fixes another issue that Nicolas fixed in bug 799818 but apparently got lost: don't optimize string[index] if the index has been out-of-bounds before to avoid frequent bailouts.
Assignee: nobody → jdemooij
Status: NEW → ASSIGNED
Attachment #824675 - Flags: review?(hv1989)
Was actually just looking into PDF.js and saw the same issue. This improves pdf.js with 4% (didn't test elaborate, so wait for awfy result.)
Blocks: 807162
Comment on attachment 824675 [details] [diff] [review]
Patch

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

Looks fine. The test for Undefined was unexpected, but does the job. Isn't Out-of-Bound access something that the baseline keeps track of? I think we only do this for writes now. But why not for reads?
Attachment #824675 - Flags: review?(hv1989) → review+
(Assignee)

Comment 4

4 years ago
(In reply to Hannes Verschore [:h4writer] from comment #3)
> Looks fine. The test for Undefined was unexpected, but does the job.

See also jsop_getelem_typed for instance.

> Isn't
> Out-of-Bound access something that the baseline keeps track of? I think we
> only do this for writes now. But why not for reads?

An out-of-bounds read will produce |undefined| (ignoring indexed properties on the prototype) so there's no need to track this separately.
(Assignee)

Comment 5

4 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/29b8e502a296

(Try: https://tbpl.mozilla.org/?tree=Try&rev=f43d7e939533)
https://hg.mozilla.org/mozilla-central/rev/29b8e502a296
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
You need to log in before you can comment on or make changes to this bug.