Closed
Bug 1343594
Opened 7 years ago
Closed 7 years ago
Baldr: update text-format warning message
Categories
(Core :: JavaScript Engine, enhancement)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla54
Tracking | Status | |
---|---|---|
firefox54 | --- | fixed |
People
(Reporter: luke, Assigned: luke)
References
(Blocks 1 open bug)
Details
Attachments
(2 files)
4.64 KB,
patch
|
yury
:
review+
|
Details | Diff | Splinter Review |
7.19 KB,
patch
|
yury
:
review+
|
Details | Diff | Splinter Review |
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).
Assignee | ||
Comment 1•7 years ago
|
||
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 2•7 years ago
|
||
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+
Assignee | ||
Comment 3•7 years ago
|
||
I noticed these are used anymore.
Attachment #8843086 -
Flags: review?(ydelendik)
Assignee | ||
Comment 4•7 years ago
|
||
*aren't
Comment 5•7 years ago
|
||
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)
Comment 7•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/d2ad3e67b80b https://hg.mozilla.org/mozilla-central/rev/c5c15cca280a
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox54:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in
before you can comment on or make changes to this bug.
Description
•