Self-host Array.prototype.filter

RESOLVED FIXED in Firefox 43

Status

()

Core
JavaScript: Standard Library
RESOLVED FIXED
3 years ago
a year ago

People

(Reporter: till, Assigned: till)

Tracking

(Depends on: 1 bug)

unspecified
mozilla43
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox43 fixed)

Details

Attachments

(3 attachments)

(Assignee)

Description

3 years ago
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

3 years ago
Created attachment 8646307 [details] [diff] [review]
Self-host Array.prototype.filter

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

3 years ago
Created attachment 8646309 [details]
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 :)
(Assignee)

Comment 5

3 years ago
Created attachment 8646606 [details] [diff] [review]
Part 0: test stack trace handling of native frames with dedicated function

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.... :(
(Assignee)

Comment 9

3 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

3 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.
https://hg.mozilla.org/mozilla-central/rev/587c76994961
https://hg.mozilla.org/mozilla-central/rev/4964d9484814
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox43: --- → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
(Assignee)

Updated

a year ago
Depends on: 1207475
You need to log in before you can comment on or make changes to this bug.