removal of JSSLOT_ARRAY_LOOKUP_HOLDER

RESOLVED FIXED

Status

()

RESOLVED FIXED
10 years ago
10 years ago

People

(Reporter: igor, Assigned: igor)

Tracking

({fixed1.9.1})

unspecified
fixed1.9.1
Points:
---
Dependency tree / graph
Bug Flags:
wanted1.9.1 +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(1 attachment)

(Assignee)

Description

10 years ago
Currently array_lookupProperty from jsarray.cpp uses a special slot, JSSLOT_ARRAY_LOOKUP_HOLDER, to store the index passed to the function. But this is not necessary since to satisfy JSObjectOps requirements it is sufficient to convert the id itself to JSProperty* and cast it back in array_dropProperty and array_getAttributes.

It not only shrinks the code but also free the slot for other uses.
(Assignee)

Comment 1

10 years ago
When creating a patch for this bug I noticed a regression that I introduced in bug 486106. There I missed that array_lookupProperty also takes care about the length property. So I will use this bug to fix that as well.

I also nominate for 1.9.1  - the bug does not affect browser or js shell, but it could bite other SpiderMonkey users.
Blocks: 486106
Severity: enhancement → normal
Flags: wanted1.9.1?
(Assignee)

Comment 2

10 years ago
Created attachment 372236 [details] [diff] [review]
v1

The essence of the patch is casting jsid to (JSProperty *) in array_lookupProperty. The rest is fixes to make sure that js_GetDenseArrayElementValue returns the correct result for any id that was accepted in array_lookupProperty. The change required to add JSContext *cx parameter to js_GetDenseArrayElementValue triggering a cascade of changes for Lookup* API in jsapi.cpp.
Attachment #372236 - Flags: review?(mrbkap)
Comment on attachment 372236 [details] [diff] [review]
v1

>Index: tm/js/src/jsarray.cpp
>  * In dense mode, holes in the array are represented by JSVAL_HOLE.  The final
>- * slot in fslots (JSSLOT_ARRAY_LOOKUP_HOLDER) is used to store the single jsid
>- * "in use" by a lookupProperty caller.
>+ * slot in fslots is unused..

Nit: extra period.
Attachment #372236 - Flags: review?(mrbkap) → review+
(Assignee)

Comment 4

10 years ago
landed to TM with the nit addressed - http://hg.mozilla.org/tracemonkey/rev/ccb029897983
Whiteboard: fixed-in-tracemonkey

Comment 5

10 years ago
http://hg.mozilla.org/mozilla-central/rev/ccb029897983
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Flags: wanted1.9.1? → wanted1.9.1+
Resolution: --- → FIXED

Updated

10 years ago
Blocks: 417501
You need to log in before you can comment on or make changes to this bug.