Closed
Bug 1334727
Opened 7 years ago
Closed 7 years ago
CacheIR: Set array length IC
Categories
(Core :: JavaScript Engine: JIT, defect, P3)
Core
JavaScript Engine: JIT
Tracking
()
RESOLVED
FIXED
mozilla54
Tracking | Status | |
---|---|---|
firefox54 | --- | fixed |
People
(Reporter: evilpie, Assigned: evilpie)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
5.78 KB,
patch
|
jandem
:
review+
|
Details | Diff | Splinter Review |
SetProp with length on arrays is probably the most common missing stub I see, except for what I assume would be solved by the add property stub. We talked about this before as well, and this probably the most common JSPropertyOp that is still relevant, so we won't need to support those. This speeds up a stupid loop running in baseline that is just assigning to length on a new array from 20ms to 7ms.
Attachment #8831382 -
Flags: review?(jdemooij)
Comment 1•7 years ago
|
||
Comment on attachment 8831382 [details] [diff] [review] CacheIR: Set array length IC Review of attachment 8831382 [details] [diff] [review]: ----------------------------------------------------------------- Great! Thanks for doing this. ::: js/src/jit/CacheIR.h @@ +756,4 @@ > Scalar::Type scalarType() { return Scalar::Type(buffer_.readByte()); } > uint32_t typeDescrKey() { return buffer_.readByte(); } > JSWhyMagic whyMagic() { return JSWhyMagic(buffer_.readByte()); } > + bool boolean() { return bool(buffer_.readByte()); } I added getBool for this in the AddSlot patch. ::: js/src/jit/VMFunctions.cpp @@ +396,5 @@ > > bool > +SetArrayLength(JSContext* cx, HandleObject obj, HandleValue value, bool strict) > +{ > + Rooted<ArrayObject*> array(cx, &obj->as<ArrayObject>()); Nit: we can eliminate the Rooted with: Handle<ArrayObject*> array = obj.as<ArrayObject>(); ::: js/src/jit/VMFunctions.h @@ +631,4 @@ > MOZ_MUST_USE bool ArrayPushDense(JSContext* cx, HandleObject obj, HandleValue v, uint32_t* length); > MOZ_MUST_USE bool ArrayShiftDense(JSContext* cx, HandleObject obj, MutableHandleValue rval); > JSString* ArrayJoin(JSContext* cx, HandleObject array, HandleString sep); > +bool SetArrayLength(JSContext* cx, HandleObject obj, HandleValue value, bool strict); Nit: add MOZ_MUST_USE for consistency.
Attachment #8831382 -
Flags: review?(jdemooij) → review+
Pushed by evilpies@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/72188279b3a9 CacheIR: SetProp array length IC. r=jandem
Updated•7 years ago
|
Priority: -- → P3
Comment 3•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/72188279b3a9
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox54:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in
before you can comment on or make changes to this bug.
Description
•