[json] add --json-parse-max-heuristic-internalized-strings [v8/v8 : main]
Joyee Cheung (Gerrit)
Joyee Cheung added 1 comment![]( "Open in Gerrit")
PTAL, this should help reducing the surface for issues similar to https://issues.chromium.org/issues/477515021 - while there are still other pathways to get to internalizations, this is one of the most commonly exercised, and it doesn't seem particularly beneficial not to put a cap on it.
Related details
- Leszek Swirski
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
chromeperf@appspot.gserviceaccount.com (Gerrit)
Message from chrom...@appspot.gserviceaccount.com![]( "Open in Gerrit")
📍 Job mac-m4-mini-perf/jetstream-main.crossbench complete.
- json-parse-inspector: base median = 548.9386108895164 -> patched median = 539.8713513261112
See results at: https://pinpoint-dot-chromeperf.appspot.com/job/1431c4ae090000
Related details
- Joyee Cheung
- Leszek Swirski
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Leszek Swirski (Gerrit)
Leszek Swirski added 2 comments![]( "Open in Gerrit")
PTAL, this should help reducing the surface for issues similar to https://issues.chromium.org/issues/477515021 - while there are still other pathways to get to internalizations, this is one of the most commonly exercised, and it doesn't seem particularly beneficial not to put a cap on it.
Agree that this doesn't hurt, but isn't it trivial to turn `["1","2","3",...]` into `{"1":0,"2":0,"3":0,...}`?
looks like this regresses json parsing 😞
Related details
- Joyee Cheung
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Joyee Cheung (Gerrit)
Joyee Cheung added 1 comment![]( "Open in Gerrit")
looks like this regresses json parsing 😞
Seems a bit surprising that this could affect the benchmark results - I imagine the default budget should be high enough to not get hit most of the time and the additional checks is relatively trivial in the entire parser. Sorry but I am not very familiar with the benchmark UI - can you point me to where it's regressing/how to reproduce so I can try to investigate what's going on?
Related details
- Leszek Swirski
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Leszek Swirski (Gerrit)
Leszek Swirski added 1 comment![]( "Open in Gerrit")
Joyee Cheunglooks like this regresses json parsing 😞
Seems a bit surprising that this could affect the benchmark results - I imagine the default budget should be high enough to not get hit most of the time and the additional checks is relatively trivial in the entire parser. Sorry but I am not very familiar with the benchmark UI - can you point me to where it's regressing/how to reproduce so I can try to investigate what's going on?
I suspect it's as simple as this path being very hot, and the additional branch causing enough overhead to be relevant. This is the json-parse-inspector benchmark from jetstream3, there's instructions on https://github.com/WebKit/JetStream/.
Related details
- Joyee Cheung
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Joyee Cheung (Gerrit)
Joyee Cheung added 1 comment![]( "Open in Gerrit")
Leszek SwirskiPTAL, this should help reducing the surface for issues similar to https://issues.chromium.org/issues/477515021 - while there are still other pathways to get to internalizations, this is one of the most commonly exercised, and it doesn't seem particularly beneficial not to put a cap on it.
Agree that this doesn't hurt, but isn't it trivial to turn `["1","2","3",...]` into `{"1":0,"2":0,"3":0,...}`?
From a runtime POV this is mostly for capping the memory usage from user JSON whose format is out of the runtime's control e.g. a server can be JSON.parsing incoming JSON payloads that can come from anywhere, and not necessarily adhere to the expected schema (some validators do other forms of parsing for validation, but it's also quite common for them to just JSON.parse on anything that passes other sanity checks e.g. total size limit, the unconditional internalizing would still allow some level of asymmetric resource usage in this case).
Related details
- Joyee Cheung
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Joyee Cheung (Gerrit)
Joyee Cheung added 1 comment![]( "Open in Gerrit")
Joyee Cheunglooks like this regresses json parsing 😞
Leszek SwirskiSeems a bit surprising that this could affect the benchmark results - I imagine the default budget should be high enough to not get hit most of the time and the additional checks is relatively trivial in the entire parser. Sorry but I am not very familiar with the benchmark UI - can you point me to where it's regressing/how to reproduce so I can try to investigate what's going on?
I suspect it's as simple as this path being very hot, and the additional branch causing enough overhead to be relevant. This is the json-parse-inspector benchmark from jetstream3, there's instructions on https://github.com/WebKit/JetStream/.
Thanks, I'll look into it. I think alternatively we can also gate it with a build config if it's the branching cost..
Related details
- Leszek Swirski
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |