788096 - Remove ObjectBox::emitLink

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Nicholas Nethercote [inactive]]

ObjectBox::emitLink can be removed with a little effort.

Comment 1

[Nicholas Nethercote [inactive]]

Attachment: Remove ObjectBox::emitLink.

This patch changes CGObjectList to use a vector instead of a list, which 
makes it simpler and more like CGConstList.  It also allows 
ObjectBox::emitLink to be removed.

Comment 2

[Nicholas Nethercote [inactive]]

Attachment: (part 2) - Introduce CGTryNoteList for storing try notes in the bytecode mitter.

This patch introduces CGTryNoteList, which is a lot like CGObjectList.  This
makes the try note handling a lot simpler.  It also removes TryNode struct,
the name of which was confusingly similar to JSTryNote.

Comment 3

[Nicholas Nethercote [inactive]]

Attachment: (part 3) - Rename GCConstList as CGConstList for consistency with similar types.

We have CGObjectList and CGTryNoteList, in which the "CG" presumably stands
for "code generation".  We also have GCConstList, which is surely a typo.  
This patch changes it to CGConstList.

Comment 4

[Jason Orendorff [:jorendorff]]

Comment on attachment 658002 [details] [diff] [review]
Remove ObjectBox::emitLink.

Converting a linked list to a vector? All right, I guess.

Comment 5

[Jason Orendorff [:jorendorff]]

Oh, I missed the other two patches here. I'll get to them tomorrow morning.

Comment 6

[Jason Orendorff [:jorendorff]]

Comment on attachment 658324 [details] [diff] [review]
(part 2) - Introduce CGTryNoteList for storing try notes in the bytecode mitter.

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

...for some definition of "morning"

::: js/src/frontend/BytecodeEmitter.cpp
@@ +6983,5 @@
> +    JSTryNote note;
> +    note.kind = kind;
> +    note.stackDepth = (uint16_t)stackDepth;
> +    note.start = (uint32_t)start;
> +    note.length = (uint32_t)(end - start);

C++-style casts (on the asserts too) would be nice.

Comment 7

[Jason Orendorff [:jorendorff]]

Comment on attachment 658326 [details] [diff] [review]
(part 3) - Rename GCConstList as CGConstList for consistency with similar types.

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

this didn't really take me 15 minutes to review, i got a coffee

Comment 8

[Nicholas Nethercote [inactive]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c77231ed11be
https://hg.mozilla.org/integration/mozilla-inbound/rev/359e4c33b89b
https://hg.mozilla.org/integration/mozilla-inbound/rev/f3b3b27099a9

Comment 9

[Nicholas Nethercote [inactive]]

I undid part 1 because it regressed Talos Trace Malloc Allocs by ~1%.  Sigh.

https://hg.mozilla.org/integration/mozilla-inbound/rev/3e1d94c1352f

Comment 10

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/c77231ed11be
https://hg.mozilla.org/mozilla-central/rev/359e4c33b89b
https://hg.mozilla.org/mozilla-central/rev/f3b3b27099a9

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/3e1d94c1352f