Closed Bug 1193280 Opened 6 years ago Closed 6 years ago

Self-host Array.prototype.filter

Categories

(Core :: JavaScript: Standard Library, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla43
Tracking Status
firefox43 --- fixed

People

(Reporter: till, Assigned: till)

References

(Depends on 1 open bug)

Details

Attachments

(3 files)

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.
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)
Attached file Benchmark
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 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+
Also, nice numbers :)
Ok, this seems to work just nicely.
Attachment #8646606 - Flags: review?(nfitzgerald)
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 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+
You forgot to re-implement Array.filter.... :(
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
(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.
https://hg.mozilla.org/mozilla-central/rev/587c76994961
https://hg.mozilla.org/mozilla-central/rev/4964d9484814
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
Depends on: 1207475
You need to log in before you can comment on or make changes to this bug.