Closed Bug 612989 Opened 9 years ago Closed 9 years ago
.parse should try to convert numbers to integer
The HTML5 Sudoku solver (IE9 test drive, bug 612930) clones boards using this method: var board = JSON.parse(JSON.stringify(inputBoard)); JSON.parse should try to convert numbers to int32; this will also help other websites and benchmarks.
This works and passes tests...
Assignee: general → jandemooij
Status: NEW → ASSIGNED
Attachment #491295 - Flags: review?(sayrer)
With that patch, is there any impact on the performance on number-heavy json parsing?
Good question; this micro-benchmark creates an array of 1000 numbers, converts it to string and then measures the time of parsing it 1000 times. -m before: 333 ms -m after: 348 ms -j before: 346 ms -j after: 354 ms v8: 433 ms So this becomes 8-15 ms slower, but this is a worst-case scenario - parsing 1000 numbers 1000 times without using the result. When you start using the numbers (like the sudoku benchmark) most operations will be faster. I will also check kraken's json-parse-financial..
Fixes a stupid bug in the micro-benchmark (used new Array(1000) and push..) -m before: 189 ms -m after: 193 ms -j before: 190 ms -j after: 194 ms v8: 315 ms So 4 ms slower now, about 2%. I don't know why the difference is smaller now though...
Attachment #491326 - Attachment is obsolete: true
This patch has no effect on Kraken's json-parse-financial (tested with -m -j -p)
Just curious... how does Opera do on those microbenchmarks? But in any case, this seems ok from that pov.
(In reply to comment #6) > Just curious... how does Opera do on those microbenchmarks? But in any case, > this seems ok from that pov. For the last test case converted to HTML: Opera 11: 55 ms Safari 5: 108 ms FF4 nightly: 183 ms Chrome 9: 370 ms With doubles: Opera 11: 125 ms Safari 5: 125 ms FF4 nightly: 218 ms Chrome 9: 422 ms For integers Opera is very fast, for doubles it's as fast as Safari. Safari becomes only slightly slower for doubles vs. integers, I wonder how they're parsing numbers...
Fwiw, Opera's json impl is generally really fast... OK, sounds like the difference in this bug is definitely a blip compared to where we need to be (the difference from Opera's numbers) at least for now.
It seems like DoubleValue is hazardous, even though we tolerate ints stored as doubles these days. Could it assert without false positives? /be
There would be false positives; there are several places where, for performance reasons, we use setDouble/DoubleValue over setNumber/NumberValue.
Comment on attachment 491295 [details] [diff] [review] Patch I agree that the concerns about perf impact in the zillion-numbers case seem unimportant. The way to win there, if it's ever needed, is to not use the slow-and-general parser when parsing a pure integer without exponent/fractional part -- some subtilties, but probably addressable ones. Maybe that's something to do whenever someone gets around to really optimizing the JSON parser -- but not here, not now, certainly.
Attachment #491295 - Flags: review?(jwalden+bmo) → review+
http://hg.mozilla.org/tracemonkey/rev/eea1f443ea15 Good stuff, thanks for the patch!
Whiteboard: [jsperf] → [jsperf] fixed-in-tracemonkey
Target Milestone: --- → mozilla2.0b8
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.