Further improve performance of JSON serialization on Linux #723
Conversation
- move expensive _SwiftValue.store case below Array, Dictionary and primitive types - add fast paths for Int and UInt types
- move NSNumber case (which uses _SwiftValue.store) below Array and Dictionary - add fast paths for Int, UInt, Float, Double
| // object is a Double and is not NaN or infinity | ||
| if let number = obj as? Double { | ||
| let invalid = number.isInfinite || number.isNaN | ||
| return !invalid |
There was a problem hiding this comment.
Would return number.isFinite work?
There was a problem hiding this comment.
That's a neater way to express it, thanks. I've updated the PR,
... when checking Double and Float validity
| case _ where _SwiftValue.store(obj) is NSNumber: | ||
| try serializeNumber(_SwiftValue.store(obj) as! NSNumber) | ||
| case let num as Int: | ||
| writer(String(describing: num)) |
There was a problem hiding this comment.
Do we really have control over how String decides to write out this number? Wouldn't it be better to keep the exact logic inside the JSONSerialization class?
There was a problem hiding this comment.
The current implementation represents the value as an NSNumber and then formats it using CFNumberFormatterCreateStringWithNumber using a CFNumberFormatter created by the JSONSerialization class. It is the CF code (and format string that is passed to the formatter) determines how a number is represented.
However, this is significantly slower than using String. Do you foresee a risk that the format of an Int / UInt produced by String may change?
An alternative could be to replicate what String is doing within JSONWriter. (is this what you had in mind?)
There was a problem hiding this comment.
It's the String(describing:) output that I'm concerned about, yes. I would rather control the output in code more tightly coupled to the writer itself, since formatting that output exactly is basically the entire job of this class.
There was a problem hiding this comment.
This isn't as straightforward as I'd expected, and I need to give it some more thought. In the meantime, I'll revert the fast paths from this PR as I'd like to deliver the 20% improvement from case reordering.
...to be reintroduced later such that JSONSerialization maintains control of the formatting
|
@parkera I've removed the Int/UInt fastpaths for now, to be submitted in a future PR, as I'd like to deliver the 20% improvement from case reordering. Are you okay with this approach? |
|
@swift-ci please test |
|
@parkera Is there anything else needed for this to be merged? |
|
This should be good to go. We should also measure again once we can get #734 landed. |
|
@swift-ci test and merge |
Following on from #718, this change further improves performance of JSON serialization:
JSONWriter.serializeJSON, move the expensive NSNumber case (which invokes_SwiftValue.store) below the cases for Array, Dictionary and primitive types.isValidJSONObjectThese changes were developed by @shmuelk and myself.
Additional performance can be gained by adding similar fast paths for Double and Float, however doing so fails the tests that expect a whole number to be serialized without decimal places (eg.
Double(1.0)->1). For that reason, I have not included a fast path for Double or Float in this PR, but I included the performance numbers below for the sake of interest.Update: I've removed the Int/UInt fastpath for now, until I can resolve the control over formatting issue raised in the review below. This PR is now just contains the 1.2x improvement (from case reordering). I'll submit the fastpaths in a future PR.
Using the benchmark I developed for #718, increased to 10,000 repetitions, I get the following results on Ubuntu 16.04:
(units are average execution time in seconds, and the code changes are cumulative).