swiftlang / swift-corelibs-foundation

The Foundation Project, providing core utilities, internationalization, and OS independence
swift.org
Apache License 2.0
5.3k stars 1.14k forks source link

[SR-5729] JSONSerialization performance regression with SR-4842 fix #4079

Open swift-ci opened 7 years ago

swift-ci commented 7 years ago
Previous ID SR-5729
Radar None
Original Reporter awishart (JIRA User)
Type Bug
Environment Ubuntu 16.04 Snapshots: 4.0-DEVELOPMENT-SNAPSHOT-2017-07-31-a, 4.0-DEVELOPMENT-SNAPSHOT-2017-08-01-a
Additional Detail from JIRA | | | |------------------|-----------------| |Votes | 2 | |Component/s | Foundation | |Labels | Bug | |Assignee | @itaiferber | |Priority | Medium | md5: a79f9fd49920ba661feefb8bed6e854e

Issue Description:

We noticed a regression in the performance of JSONSerialization on Linux between 07-31-a and 08-01-a Swift 4.0 snapshots and believe we have narrowed it down to the fix implemented for SR-4842. The regression can be seen by running the microbenchmark here: http://swift.sandbox.bluemix.net/#/repl/599bf6a1dcc75d533468b919 against the 07-31-a snapshot and again against the same snapshot rebuilt with this change (https://github.com/apple/swift-corelibs-foundation/commit/d1d1538eed7bb005c34d12a30e03b5927c040bfb) applied.

Example output from the microbenchmark:

gruffalo:~/swift-benchmarks$ ./jsonEncoderTest_0731
JSONSerialization (Array) took 12942.009 ns
gruffalo:~/swift-benchmarks$ ./jsonEncoderTest_0731_suspect
JSONSerialization (Array) took 17908.626 ns

In this instance, serialising an array of 20 values is about 30-40% slower.

belkadan commented 7 years ago

@itaiferber?

itaiferber commented 7 years ago

I'll try to get a Linux build to test and profile.

swift-ci commented 7 years ago

Comment by Alex Wishart (JIRA)

Any news on this @itaiferber ?

itaiferber commented 7 years ago

awishart (JIRA User) I didn't manage to get a Linux box up and running, and I've been busy since. I'll try again soon if I can. That being said, the change for SR-4842 was a correctness fix which now involves wrapping and unwrapping more {{Optional}}s. It's expected that more work is going on, but I'm not sure how big a performance impact this has in the real world.