Skip to content

[4.2] Cherry pick: skip allocation if reserve capacity is smol #16807

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 2 commits into from
May 24, 2018
Merged

[4.2] Cherry pick: skip allocation if reserve capacity is smol #16807

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
674b466
[benchmark] Add reserveCapacity String benchmark to builder.
milseman May 17, 2018
aa8b06f
[string] Skip allocation in reserveCapacity if smol
milseman May 17, 2018
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
17 changes: 16 additions & 1 deletion benchmark/single-source/StringBuilder.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ public let StringBuilder = [
name: "StringBuilder",
runFunction: run_StringBuilder,
tags: [.validation, .api, .String]),
BenchmarkInfo(
name: "StringBuilderSmallReservingCapacity",
runFunction: run_StringBuilderSmallReservingCapacity,
tags: [.validation, .api, .String]),
BenchmarkInfo(
name: "StringUTF16Builder",
runFunction: run_StringUTF16Builder,
Expand Down Expand Up @@ -48,8 +52,11 @@ public let StringBuilder = [
]

@inline(never)
func buildString(_ i: String) -> String {
func buildString(_ i: String, reservingCapacity: Bool = false) -> String {
var sb = getString(i)
if reservingCapacity {
sb.reserveCapacity(10)
}
for str in ["b","c","d","pizza"] {
sb += str
}
Expand All @@ -63,6 +70,13 @@ public func run_StringBuilder(_ N: Int) {
}
}

@inline(never)
public func run_StringBuilderSmallReservingCapacity(_ N: Int) {
for _ in 1...5000*N {
blackHole(buildString("a", reservingCapacity: true))
}
}

@inline(never)
func addString(_ i: String) -> String {
let s = getString(i) + "b" + "c" + "d" + "pizza"
Expand Down Expand Up @@ -179,3 +193,4 @@ public func run_StringWordBuilderReservingCapacity(_ N: Int) {
blackHole(buildString(
word: "bumfuzzle", count: 50_000 * N, reservingCapacity: true))
}

10 changes: 7 additions & 3 deletions stdlib/public/core/StringGuts.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -966,18 +966,22 @@ extension _StringGuts {
}
}

// TODO (TODO: JIRA): check if we're small and still within capacity
// Small strings can accomodate small capacities
if capacity <= _SmallUTF8String.capacity {
return
}

let selfCount = self.count
if isASCII {
let storage = _copyToNativeStorage(
of: UInt8.self,
from: 0..<self.count,
from: 0..<selfCount,
unusedCapacity: Swift.max(capacity - count, 0))
self = _StringGuts(_large: storage)
} else {
let storage = _copyToNativeStorage(
of: UTF16.CodeUnit.self,
from: 0..<self.count,
from: 0..<selfCount,
unusedCapacity: Swift.max(capacity - count, 0))
self = _StringGuts(_large: storage)
}
Expand Down
14 changes: 6 additions & 8 deletions test/stdlib/NewStringAppending.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,27 +69,25 @@ var s = "⓪" // start non-empty

// To make this test independent of the memory allocator implementation,
// explicitly request initial capacity.
s.reserveCapacity(8)
s.reserveCapacity(16)

// CHECK-NEXT: String(Native(owner: @[[storage0:[x0-9a-f]+]], count: 1, capacity: 8)) = "⓪"
// CHECK-NEXT: String(Native(owner: @[[storage0:[x0-9a-f]+]], count: 1, capacity: 16)) = "⓪"
print("\(repr(s))")

// CHECK-NEXT: String(Native(owner: @[[storage0]], count: 2, capacity: 8)) = "⓪1"
// CHECK-NEXT: String(Native(owner: @[[storage0]], count: 2, capacity: 16)) = "⓪1"
s += "1"
print("\(repr(s))")

// CHECK-NEXT: String(Native(owner: @[[storage0]], count: 8, capacity: 8)) = "⓪1234567"
// CHECK-NEXT: String(Native(owner: @[[storage0]], count: 8, capacity: 16)) = "⓪1234567"
s += "234567"
print("\(repr(s))")

// -- expect a reallocation here

// CHECK-NEXT: String(Native(owner: @[[storage1:[x0-9a-f]+]], count: 9, capacity: 16)) = "⓪12345678"
// CHECK-NEXT: String(Native(owner: @[[storage0:[x0-9a-f]+]], count: 9, capacity: 16)) = "⓪12345678"
// CHECK-NOT: @[[storage0]],
s += "8"
print("\(repr(s))")

// CHECK-NEXT: String(Native(owner: @[[storage1]], count: 16, capacity: 16)) = "⓪123456789012345"
// CHECK-NEXT: String(Native(owner: @[[storage0]], count: 16, capacity: 16)) = "⓪123456789012345"
s += "9012345"
print("\(repr(s))")

Expand Down
10 changes: 5 additions & 5 deletions validation-test/stdlib/String.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -948,7 +948,7 @@ StringTests.test("stringGutsReserve")
base._guts._objectIdentifier != nil &&
isUnique

base.reserveCapacity(0)
base.reserveCapacity(16)
// Now it's unique

// If it was already native and unique, no reallocation
Expand Down Expand Up @@ -1065,11 +1065,11 @@ StringTests.test("reserveCapacity") {
expectNotEqual(id0, s.bufferID)
s = ""
print("empty capacity \(s.capacity)")
s.reserveCapacity(oldCap + 2)
print("reserving \(oldCap + 2) -> \(s.capacity), width = \(s._guts.byteWidth)")
s.reserveCapacity(oldCap + 18)
print("reserving \(oldCap + 18) -> \(s.capacity), width = \(s._guts.byteWidth)")
let id1 = s.bufferID
s.insert(contentsOf: repeatElement(x, count: oldCap + 2), at: s.endIndex)
print("extending by \(oldCap + 2) -> \(s.capacity), width = \(s._guts.byteWidth)")
s.insert(contentsOf: repeatElement(x, count: oldCap + 18), at: s.endIndex)
print("extending by \(oldCap + 18) -> \(s.capacity), width = \(s._guts.byteWidth)")
expectEqual(id1, s.bufferID)
s.insert(contentsOf: repeatElement(x, count: s.capacity + 100), at: s.endIndex)
expectNotEqual(id1, s.bufferID)
Expand Down