Baldr: have DecodeModuleEnvironment start the code section

RESOLVED FIXED in Firefox 58

Status

()

enhancement
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: luke, Assigned: luke)

Tracking

unspecified
mozilla58
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox58 fixed)

Details

Attachments

(2 attachments)

For real streaming compilation (bug 1347644), I think we want to buffer up enough bytes to decode the module environment before starting a compilation helper task to do a streaming decode of the code section.  To do a streaming decode, we want to know the code section size so that the memory can be allocated and not need to be resized during parallel compilation.  This implies having DecodeModuleEnvironment() startSection(SectionId::Code).  The patches end up simplifying the code too.
The next patch ended up suggesting a refactoring I should've made to begin with: putting (sectionStart, sectionSize) into a Maybe<Pair> that is returned as an outparam in startSection() and given to finishSection().
Attachment #8915115 - Flags: review?(lhansen)
This patch does what comment 0 said, sticking a MaybeSectionRange in the ModuleEnvironment which ends up obviating the need for peekSectionSize().
Attachment #8915116 - Flags: review?(lhansen)
Comment on attachment 8915115 [details] [diff] [review]
tidy-start-section

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

::: js/src/wasm/WasmValidate.h
@@ +25,5 @@
>  namespace js {
>  namespace wasm {
>  
> +// This struct captures the bytecode offset of a section's payload (so not
> +// including the header) and the section size of the payload.

This should probably be just "the size of that payload".
Attachment #8915115 - Flags: review?(lhansen) → review+
Comment on attachment 8915116 [details] [diff] [review]
start-code-section-in-decode-module-env

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

Nice.
Attachment #8915116 - Flags: review?(lhansen) → review+
Pushed by lwagner@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/62fc2f51d566
Baldr: stidy Decoder::startSection (r=lth)
https://hg.mozilla.org/integration/mozilla-inbound/rev/13b556ec5a8e
Baldr: decode Code section header in DecodeModuleEnvironment (r=lth)
https://hg.mozilla.org/mozilla-central/rev/62fc2f51d566
https://hg.mozilla.org/mozilla-central/rev/13b556ec5a8e
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
You need to log in before you can comment on or make changes to this bug.