Closed
Bug 1193280
Opened 9 years ago
Closed 9 years ago
Self-host Array.prototype.filter
Categories
(Core :: JavaScript: Standard Library, defect)
Core
JavaScript: Standard Library
Tracking
()
RESOLVED
FIXED
mozilla43
Tracking | Status | |
---|---|---|
firefox43 | --- | fixed |
People
(Reporter: till, Assigned: till)
References
Details
Attachments
(3 files)
5.91 KB,
patch
|
jandem
:
review+
fitzgen
:
feedback+
|
Details | Diff | Splinter Review |
367 bytes,
application/x-javascript
|
Details | |
3.33 KB,
patch
|
fitzgen
:
review+
|
Details | Diff | Splinter Review |
Now that we have a fast way of defining data propertiers at least for array elements, this is straight-forward. It's also 7.5x as fast as what we currently have on the benchmark I'll attach.
Assignee | ||
Comment 1•9 years ago
|
||
Nick, this causes jit-test/tests/saved-stacks/native-calls.js to fail because Array#filter is self-hosted now. I can't think of a good replacement right now, so I'm unsure how to keep this test. Perhaps we need a test function that takes a callback and use that?
Attachment #8646307 -
Flags: review?(jdemooij)
Attachment #8646307 -
Flags: feedback?(nfitzgerald)
Assignee | ||
Comment 2•9 years ago
|
||
This calls filter on two different objects with two different callbacks, to ensure that we don't neglect the influence of polymorphism. It doesn't seem to matter in this case, as the resulting times are 2x compared to only calling filter on one of the arrays. Numbers: SM old: 1687ms SM new: 224ms d8: 1949ms JSC: 482ms Pretty nice, I'd say.
Comment 3•9 years ago
|
||
Comment on attachment 8646307 [details] [diff] [review] Self-host Array.prototype.filter Review of attachment 8646307 [details] [diff] [review]: ----------------------------------------------------------------- Yeah, I think a new TestingFunction that just calls the provided JS callback with its native frame on the stack is best.
Attachment #8646307 -
Flags: feedback?(nfitzgerald) → feedback+
Comment 4•9 years ago
|
||
Also, nice numbers :)
Assignee | ||
Comment 5•9 years ago
|
||
Ok, this seems to work just nicely.
Attachment #8646606 -
Flags: review?(nfitzgerald)
Comment 6•9 years ago
|
||
Comment on attachment 8646606 [details] [diff] [review] Part 0: test stack trace handling of native frames with dedicated function Review of attachment 8646606 [details] [diff] [review]: ----------------------------------------------------------------- Thanks!
Attachment #8646606 -
Flags: review?(nfitzgerald) → review+
Comment 7•9 years ago
|
||
Comment on attachment 8646307 [details] [diff] [review] Self-host Array.prototype.filter Review of attachment 8646307 [details] [diff] [review]: ----------------------------------------------------------------- Great to get rid of another slow C++ -> JS call.
Attachment #8646307 -
Flags: review?(jdemooij) → review+
Comment 8•9 years ago
|
||
You forgot to re-implement Array.filter.... :(
Assignee | ||
Comment 9•9 years ago
|
||
url: https://hg.mozilla.org/integration/mozilla-inbound/rev/587c7699496123e8fa994a5e4f7addf8bd6649b0 changeset: 587c7699496123e8fa994a5e4f7addf8bd6649b0 user: Till Schneidereit <till@tillschneidereit.net> date: Fri Aug 14 13:05:26 2015 +0200 description: Bug 1193280 - Part 1: test stack trace handling of native frames with dedicated function. r=fitzgen url: https://hg.mozilla.org/integration/mozilla-inbound/rev/4964d9484814e44d717e7315a408c4098486b364 changeset: 4964d9484814e44d717e7315a408c4098486b364 user: Till Schneidereit <till@tillschneidereit.net> date: Fri Aug 14 13:13:36 2015 +0200 description: Bug 1193280 - Part 2: Self-host Array.prototype.filter. r=jandem
Assignee | ||
Comment 10•9 years ago
|
||
(In reply to Tom Schuster [:evilpie] from comment #8) > You forgot to re-implement Array.filter.... :( Thanks, while I didn't see your comment until just now, I did see the test failure that caused in time, and fixed it before landing.
Comment 11•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/587c76994961 https://hg.mozilla.org/mozilla-central/rev/4964d9484814
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox43:
--- → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
You need to log in
before you can comment on or make changes to this bug.
Description
•