New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Further improve performance of JSON serialization on Linux #723

Merged
merged 4 commits into from Dec 12, 2016

Conversation

Projects
None yet
4 participants
@djones6
Contributor

djones6 commented Nov 23, 2016

Following on from #718, this change further improves performance of JSON serialization:

  • In JSONWriter.serializeJSON, move the expensive NSNumber case (which invokes _SwiftValue.store) below the cases for Array, Dictionary and primitive types.
  • add fast paths for Int and UInt types, which can be converted directly to a String rather than going via NSNumber.
  • Equivalent changes for isValidJSONObject

These 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:

Payload type:       Int   (speedup)        Double   (speedup)

Baseline:           8.72                   8.78
With #718:          4.18  (2.1x)           3.97   (2.2x)
Reorder case:       3.43  (1.2x)           3.29   (1.2x)
Int/UInt fastpath:  2.33  (1.5x)           ----

(Double fastpath):  ----                   2.42   (1.35x)

(units are average execution time in seconds, and the code changes are cumulative).

djones6 added some commits Nov 23, 2016

Improve performance of JSON serialization
- move expensive _SwiftValue.store case below Array, Dictionary and primitive types

- add fast paths for Int and UInt types
Improve performance of isValidJSONObject
- move NSNumber case (which uses _SwiftValue.store) below Array and Dictionary

- add fast paths for Int, UInt, Float, Double
Replace negation of isInfinite or isNaN with isFinite
... when checking Double and Float validity
Revert Int/UInt fast paths
...to be reintroduced later such that JSONSerialization maintains control of the formatting
@djones6

This comment has been minimized.

Show comment
Hide comment
@djones6

djones6 Dec 2, 2016

Contributor

@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?

Contributor

djones6 commented Dec 2, 2016

@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?

@parkera

This comment has been minimized.

Show comment
Hide comment
@parkera

parkera Dec 2, 2016

Member

@swift-ci please test

Member

parkera commented Dec 2, 2016

@swift-ci please test

@djones6

This comment has been minimized.

Show comment
Hide comment
@djones6

djones6 Dec 12, 2016

Contributor

@parkera Is there anything else needed for this to be merged?

Contributor

djones6 commented Dec 12, 2016

@parkera Is there anything else needed for this to be merged?

@parkera

This comment has been minimized.

Show comment
Hide comment
@parkera

parkera Dec 12, 2016

Member

This should be good to go. We should also measure again once we can get #734 landed.

Member

parkera commented Dec 12, 2016

This should be good to go. We should also measure again once we can get #734 landed.

@parkera

This comment has been minimized.

Show comment
Hide comment
@parkera

parkera Dec 12, 2016

Member

@swift-ci test and merge

Member

parkera commented Dec 12, 2016

@swift-ci test and merge

@swift-ci swift-ci merged commit e4aef0f into apple:master Dec 12, 2016

1 of 2 checks passed

Test and Merge Build started.
Details
Swift Test Linux Platform Build finished.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment