Closed Bug 1343594 Opened 7 years ago Closed 7 years ago

Baldr: update text-format warning message

Categories

(Core :: JavaScript Engine, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla54
Tracking Status
firefox54 --- fixed

People

(Reporter: luke, Assigned: luke)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

I think we can now take off the ASCII-art experimental text format message.  Instead, until we do source-per-function, I think we should print a warning when the source is huge (explaining the issues are known and intended to be fixed).
Attached patch warning-messageSplinter Review
I started by writing a patch that actually cut off after N lines, but even for really small N, after the BinaryToText() was done, there would be a hang in debugger.js.  Anyhow, this patch just disables it entirely for modules with bytecode bigger than 1mb.
Attachment #8842647 - Flags: review?(ydelendik)
Comment on attachment 8842647 [details] [diff] [review]
warning-message

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

Looks good.

::: js/src/wasm/WasmCode.cpp
@@ +735,5 @@
>  {
>      StringBuffer buffer(cx);
> +    if (maybeBytecode_ && maybeBytecode_->bytes.length() > TooBig) {
> +        if (!buffer.append(tooBigMessage))
> +            return nullptr;

can you add something like `MOZ_ASSERT(!maybeSourceMap_);` just to indicate we are keeping source map not set?
Attachment #8842647 - Flags: review?(ydelendik) → review+
Attached patch rm-func-locsSplinter Review
I noticed these are used anymore.
Attachment #8843086 - Flags: review?(ydelendik)
*aren't
Comment on attachment 8843086 [details] [diff] [review]
rm-func-locs

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

Thanks

::: js/src/wasm/WasmCode.cpp
@@ +771,2 @@
>  #if DEBUG
>          // Checking source map invariant: expression and function locations must be sorted

Can you remove function locations reference from the comment?
Attachment #8843086 - Flags: review?(ydelendik) → review+
Pushed by lwagner@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d2ad3e67b80b
Baldr: remove dead functionlocs (r=yury)
https://hg.mozilla.org/integration/mozilla-inbound/rev/c5c15cca280a
Baldr: issue warning for binary-to-text on huge modules (r=yury)
https://hg.mozilla.org/mozilla-central/rev/d2ad3e67b80b
https://hg.mozilla.org/mozilla-central/rev/c5c15cca280a
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in before you can comment on or make changes to this bug.