Boris Zbarsky [:bzbarsky] Reporter
	Description • 10 years ago

We should have it for ArrayClass bits.

	Claude Pache
	Comment 1 • 9 years ago

Isn't it a dupe of Bug 1054758 ?

	Boris Zbarsky [:bzbarsky] Reporter
	Comment 2 • 9 years ago

Yes, except filed a month earlier.  So the dup is the other way around.  ;)

Not that I care much as long as the dependencies are propagated to whichever bug is used here.

	Tom S [:evilpie] Assignee
	Comment 4 • 9 years ago

Changing the implementation of concat for this is really simple. However concat is also inlined in Ion, and just checking for the array class isn't enough anymore.

	Boris Zbarsky [:bzbarsky] Reporter
	Comment 5 • 9 years ago

As pointed out on public-script-coord, we should make @@isConcatSpreadable return undefined on cross-origin windows when we implement this.

	Tom S [:evilpie] Assignee
	Comment 6 • 8 years ago

This should be relatively simple with the new selfhosted concat.

	Tom S [:evilpie] Assignee
	Comment 7 • 8 years ago

I was a bit upset that we actually hard-coded @@iterator as the first symbol, so I decided to automatically generate all the symbol atoms. So now you can add new symbols at the frond and end of the enum!

	Tom S [:evilpie] Assignee
	Comment 9 • 8 years ago

Normally we would be done now, sadly this causes a regression between 10-25% on a micro benchmark using concat. It seems like the O[@@isConcatSpreadable] lookup is not optimized away and even uses a property cache. I will have to look into completely eliminating that.

	Jason Orendorff [:jorendorff]
	Comment 11 • 8 years ago

Comment on attachment 8739665 [details] [diff] [review]
Autogenerate symbol names

Review of attachment 8739665 [details] [diff] [review]:
-----------------------------------------------------------------

Yes, of course. Thanks.

	Pulsebot
	Comment 12 • 8 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/aceaeee7103f

	Tom S [:evilpie] Assignee
	Comment 13 • 8 years ago

This patch allows us to completely fold away O[Symbol.isContactSpreadable], this wins back all the performance on a micro benchmark. I tried running and didn't see any obvious, but the results are extremely noisy for me.

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 14 • 8 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/aceaeee7103f

	Jan de Mooij [:jandem]
	Comment 15 • 8 years ago

Comment on attachment 8741039 [details] [diff] [review]
Fold away property accesses to not-defined properties

Review of attachment 8741039 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks, that's a nice optimization to have for other code as well.

	Pulsebot
	Comment 16 • 8 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/410d733097ac

	Carsten Book [:Tomcat]
	Comment 17 • 8 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/410d733097ac

	Tooru Fujisawa [:arai]
	Comment 18 • 8 years ago

Comment on attachment 8739669 [details] [diff] [review]
IsConcatSpreadable

Review of attachment 8739669 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, assuming the testcase is in another patch :)

	Tooru Fujisawa [:arai]
	Comment 20 • 8 years ago

Comment on attachment 8741906 [details] [diff] [review]
Tests

Review of attachment 8741906 [details] [diff] [review]:
-----------------------------------------------------------------

Nice testcases :D

	Tom S [:evilpie] Assignee
	Comment 21 • 8 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/db1b027cbeb1
https://hg.mozilla.org/integration/mozilla-inbound/rev/2021fd52a8c9
https://hg.mozilla.org/integration/mozilla-inbound/rev/35b1a010cb9f

	Carsten Book [:Tomcat]
	Comment 22 • 8 years ago

sorry had to back this out for assertion failures like https://treeherder.mozilla.org/logviewer.html#?job_id=25962392&repo=mozilla-inbound

	Tom S [:evilpie] Assignee
	Comment 23 • 8 years ago

I think that test might be buggy sometimes? Try looked okay https://treeherder.mozilla.org/#/jobs?repo=try&revision=1c3b1879918a actually.

	Pulsebot
	Comment 24 • 8 years ago

Backout:
https://hg.mozilla.org/mozilla-central/rev/533daf1ab8c9
https://hg.mozilla.org/mozilla-central/rev/fe492b1cbc39
https://hg.mozilla.org/mozilla-central/rev/077bd7786468

	Tom S [:evilpie] Assignee
	Comment 25 • 8 years ago

I am just going to assume this test is broken, inserting a print makes the failure go away.

	Tom S [:evilpie] Assignee
	Comment 26 • 8 years ago

It seems like this test case has been causing problems for a long time. I have to admit, I am not interested in fixing this just to get this bug finished.

	Jan de Mooij [:jandem]
	Comment 27 • 8 years ago

Comment on attachment 8742864 [details] [diff] [review]
Prevent jit compilation for one expression-autopsy test

Review of attachment 8742864 [details] [diff] [review]:
-----------------------------------------------------------------

I'm still a bit curious why your patch exposes this, but given that we have seen this failure before and investigated it last time, this makes sense.

	Pulsebot
	Comment 28 • 8 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/bb2748e05cab
https://hg.mozilla.org/integration/mozilla-inbound/rev/1ddd1d9af872
https://hg.mozilla.org/integration/mozilla-inbound/rev/219bb598b6ad
https://hg.mozilla.org/integration/mozilla-inbound/rev/f569192eb5e8

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 29 • 8 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/bb2748e05cab
https://hg.mozilla.org/mozilla-central/rev/1ddd1d9af872
https://hg.mozilla.org/mozilla-central/rev/219bb598b6ad
https://hg.mozilla.org/mozilla-central/rev/f569192eb5e8

	Florian Scholz (Open Web Docs)
	Comment 30 • 8 years ago

Developer release notes
https://developer.mozilla.org/en-US/Firefox/Releases/48

Reference pages:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Symbol
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Symbol/isConcatSpreadable