1364615 - Baldr: tidy up some things about wasm::Code

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement)

Description

[Luke Wagner [:luke]]

A couple small cleanup patches.

Comment 1

[Luke Wagner [:luke]]

Attachment: unique-code-bytes

This patch adds a new UniquePtr typedef that releases code which makes ownership of code more foolproof.

Comment 2

[Luke Wagner [:luke]]

Attachment: rm-unlink

Comment 3

[Luke Wagner [:luke]]

(Oops, hit enter too soon.)  The patch in comment 2 avoids the somewhat strange unlinkedBytesForDebugging() by instead making a copy of the bytes directly from the MacroAssembler before linking.  Also should be a tiny bit faster.

Comment 4

[Luke Wagner [:luke]]

Attachment: rm-alloc-limit

We're no longer doing mixed X/W mappings so, as the comment says...

Comment 5

[Luke Wagner [:luke]]

Attachment: rm-code-bytecode

This patch removes maybeBytecode from wasm::Code since wasm::Code basically has no need for it.  Instead, the 1 use (passing maybeBytecode_ to Metadata::getFuncName) is done in Instance instead which already has access to maybeBytecode via debug_.  This keeps wasm::Code more focused on just executable code, which I think is better.

Comment 6

[Jan de Mooij [:jandem]]

Comment on attachment 8867411 [details] [diff] [review]
rm-alloc-limit

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

Nice.

Comment 7

[Lars T Hansen [:lth]]

Comment on attachment 8867409 [details] [diff] [review]
unique-code-bytes

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

::: js/src/wasm/WasmCode.h
@@ +57,5 @@
>  typedef UniquePtr<const CodeSegment> UniqueConstCodeSegment;
>  
>  class CodeSegment
>  {
> +    // Executable code must be release deallocated specially.

Presumably "deallocated" is all you mean and "release" should be removed.

Comment 8

[Lars T Hansen [:lth]]

Comment on attachment 8867412 [details] [diff] [review]
rm-code-bytecode

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

Nice.  I strongly favor this, (even if it will interact in an ugly way with a long patch queue I have).

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/94bc2a2c7274
Baldr: use UniquePtr to own code (r=lth)
https://hg.mozilla.org/integration/mozilla-inbound/rev/b8bbd7e9579a
Baldr: remove unlinkedBytesForDebugging (r=lth)
https://hg.mozilla.org/integration/mozilla-inbound/rev/4901d885b1a4
Baldr: remove MaxWasmCodeAllocations (r=jandem)
https://hg.mozilla.org/integration/mozilla-inbound/rev/2e27f56d21b4
Baldr: remove bytecode from wasm::Code (r=lth)

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/94bc2a2c7274
https://hg.mozilla.org/mozilla-central/rev/b8bbd7e9579a
https://hg.mozilla.org/mozilla-central/rev/4901d885b1a4
https://hg.mozilla.org/mozilla-central/rev/2e27f56d21b4