Skip to content

Added fast path for Int and UInt #923

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Apr 11, 2017
Merged

Added fast path for Int and UInt #923

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
712f349
Added fast path for Int and UInt
Mar 23, 2017
78de7ea
Make reserveCapacity values more readable & test fixes
Mar 27, 2017
313cc93
Fix a typo in the tests
Mar 28, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 92 additions & 0 deletions Foundation/NSJSONSerialization.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,8 @@ internal extension JSONSerialization {
//MARK: - JSONSerializer
private struct JSONWriter {

private let maxUIntLength = String(describing: UInt.max).characters.count
private let maxIntLength = String(describing: Int.max).characters.count
var indent = 0
let pretty: Bool
let writer: (String?) -> Void
Expand All @@ -309,6 +311,10 @@ private struct JSONWriter {
try serializeString(str)
case let boolValue as Bool:
serializeBool(boolValue)
case let num as Int:
try serializeInt(value: num)
case let num as UInt:
try serializeUInt(value: num)
case let array as Array<Any>:
try serializeArray(array)
case let dict as Dictionary<AnyHashable, Any>:
Expand All @@ -322,6 +328,92 @@ private struct JSONWriter {
}
}

private func serializeUInt(value: UInt) throws {
if value == 0 {
writer("0")
return
}
var array: [UInt] = []
var stringResult = ""
//Maximum length of an UInt
array.reserveCapacity(maxUIntLength)
stringResult.reserveCapacity(maxUIntLength)
var number = value

while number != 0 {
array.append(number % 10)
number /= 10
}

/*
Step backwards through the array and append the values to the string. This way the values are appended in the correct order.
*/
var counter = array.count
while counter > 0 {
counter -= 1
let digit: UInt = array[counter]
switch digit {
case 0: stringResult.append("0")
case 1: stringResult.append("1")
case 2: stringResult.append("2")
case 3: stringResult.append("3")
case 4: stringResult.append("4")
case 5: stringResult.append("5")
case 6: stringResult.append("6")
case 7: stringResult.append("7")
case 8: stringResult.append("8")
case 9: stringResult.append("9")
default: fatalError()
}
}

writer(stringResult)
}

private func serializeInt(value: Int) throws {
if value == 0 {
writer("0")
return
}
var array: [Int] = []
var stringResult = ""
array.reserveCapacity(maxIntLength)
//Account for a negative sign
stringResult.reserveCapacity(maxIntLength + 1)
var number = value

while number != 0 {
array.append(number % 10)
number /= 10
}
//If negative add minus sign before adding any values
if value < 0 {
stringResult.append("-")
}
/*
Step backwards through the array and append the values to the string. This way the values are appended in the correct order.
*/
var counter = array.count
while counter > 0 {
counter -= 1
let digit = array[counter]
switch digit {
case 0: stringResult.append("0")
case 1, -1: stringResult.append("1")
case 2, -2: stringResult.append("2")
case 3, -3: stringResult.append("3")
case 4, -4: stringResult.append("4")
case 5, -5: stringResult.append("5")
case 6, -6: stringResult.append("6")
case 7, -7: stringResult.append("7")
case 8, -8: stringResult.append("8")
case 9, -9: stringResult.append("9")
default: fatalError()
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Though the loops where we walk backwards are not exactly identical in the two functions, I am wondering if we can still refactor them into a single function.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the feedback, it is possible to refactor them into a single function however both Ian and myself felt the code looked more complicated and required a little more work in terms of casting and such, so I've omitted it for now.

writer(stringResult)
}

func serializeString(_ str: String) throws {
writer("\"")
for scalar in str.unicodeScalars {
Expand Down
24 changes: 24 additions & 0 deletions TestFoundation/TestNSJSONSerialization.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -863,6 +863,10 @@ extension TestNSJSONSerialization {
("test_nested_array", test_nested_array),
("test_nested_dictionary", test_nested_dictionary),
("test_serialize_number", test_serialize_number),
("test_serialize_IntMax", test_serialize_IntMax),
("test_serialize_IntMin", test_serialize_IntMin),
("test_serialize_UIntMax", test_serialize_UIntMax),
("test_serialize_UIntMin", test_serialize_UIntMin),
("test_serialize_stringEscaping", test_serialize_stringEscaping),
("test_jsonReadingOffTheEndOfBuffers", test_jsonReadingOffTheEndOfBuffers),
("test_jsonObjectToOutputStreamBuffer", test_jsonObjectToOutputStreamBuffer),
Expand Down Expand Up @@ -1054,6 +1058,26 @@ extension TestNSJSONSerialization {
XCTAssertEqual(try trySerialize(json), "[false,true]")
}

func test_serialize_IntMax() {
let json: [Any] = [Int.max]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is Int.max architecture specific?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Documentation:

On 32-bit platforms, Int is the same size as Int32, and on 64-bit platforms, Int is the same size as Int64.

So I think this will break on 32-bit platforms. We should construct the string we're asserting against using the same constant, I think. The same applies for the tests which use Int.min, UInt.max and UInt.min.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for finding that, I've updated the tests to compare against a string representation of the Int.max value so this should also work on 32-bit platforms now.

XCTAssertEqual(try trySerialize(json), "[\(Int.max)]")
}

func test_serialize_IntMin() {
let json: [Any] = [Int.min]
XCTAssertEqual(try trySerialize(json), "[\(Int.min)]")
}

func test_serialize_UIntMax() {
let json: [Any] = [UInt.max]
XCTAssertEqual(try trySerialize(json), "[\(UInt.max)]")
}

func test_serialize_UIntMin() {
let json: [Any] = [UInt.min]
XCTAssertEqual(try trySerialize(json), "[\(UInt.min)]")
}

func test_serialize_stringEscaping() {
var json = ["foo"]
XCTAssertEqual(try trySerialize(json), "[\"foo\"]")
Expand Down