repl: printing big strings results in OOM #25478
Comments
joyeecheung
added
repl
|
Is (part) of the issue that we write out a flattened string to the stream? |
|
@addaleax Sort of, we use I am not sure what's the best way to work around this, or if it even should be fixed. It doesn't seem unreasonable for V8 to create a temporary copy to flatten the string and calculate the length of the string encoded in UTF8. Even if we somehow manage to avoid the flattening of the string that triggers a temporary copy in Another fun observation - if I configure node with |
|
To me this looks like a known limitation and I would not know how to overcome this problem. |
|
@joyeecheung @addaleax is this something we should really look into at the moment? I would otherwise suggest to close this. If someone wants to improve this somehow, that's awesome but I think things work relatively well for now that it's not really required? |
|
@BridgeAR I wouldn’t think this counts as working well (this should throw instead of crash), but yeah, there is no plan fixing this - not because this is a wontfix but more like we don’t know how to fix this. So I guess we can add helped wanted and see if there’s help. |
joyeecheung
added
the
help wanted
|
@joyeecheung @BridgeAR Would it make sense to try and add a V8 API to read a cons string into multiple Buffers? I’m not sure if it helps here, but it would make things like this easier? |
|
Thinking about this, I think it’s actually pretty unreasonable for |
|
@addaleax Yeah, I think once the length of the string exceeds a certain value it's not really useful to display the whole string any more. The issue is before deciding whether to truncate the string, we first need to know at least the length of it, which requires flattening (and thus copying) of cons strings. Maybe one way to go about it is to implement a V8 API that stops the flattening once a specified maximum length is reached, |
|
It should be possible to teach Right now they always call There might already be (partial?) support for that in the |
Refs: #25478 PR-URL: #32392 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Matheus Marchini <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: James M Snell <[email protected]>
Refs: #25478 PR-URL: #32392 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Matheus Marchini <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: James M Snell <[email protected]>
Refs: #25478 PR-URL: #32392 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Matheus Marchini <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: James M Snell <[email protected]>
Refs: nodejs#25478 PR-URL: nodejs#32392 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Matheus Marchini <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: James M Snell <[email protected]>
Refs: #25478 PR-URL: #32392 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Matheus Marchini <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: James M Snell <[email protected]>
|
This is fixed in newer Node.js versions by limiting the string inspection. |
From https://bugs.chromium.org/p/v8/issues/detail?id=8679
How to reproduce:
Reposting my comment in the v8 issue:
Stack trace:
Top frames of
_v8_internal_Print_StackTrace()The text was updated successfully, but these errors were encountered: