Skip to content

Commit

Permalink
Merge pull request swiftlang#30244 from milseman/string_shrink
Browse files Browse the repository at this point in the history
[string] Shrink storage class sizes
  • Loading branch information
milseman authored Mar 6, 2020
2 parents be141e6 + 89f0a12 commit 79bac4e
Show file tree
Hide file tree
Showing 10 changed files with 410 additions and 199 deletions.
8 changes: 8 additions & 0 deletions stdlib/public/core/LegacyABI.swift
Original file line number Diff line number Diff line change
Expand Up @@ -79,3 +79,11 @@ extension String {
range: Range<String.Index>
) { fatalError() }
}

extension String.UTF16View {
// Swift 5.x: This was accidentally shipped as inlinable, but was never used
// from an inlinable context. The definition is kept around for techincal ABI
// compatibility (even though it shouldn't matter), but is unused.
@inlinable @inline(__always)
internal var _shortHeuristic: Int { return 32 }
}
44 changes: 5 additions & 39 deletions stdlib/public/core/StringBreadcrumbs.swift
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,16 @@

// @opaque
internal final class _StringBreadcrumbs {
static var breadcrumbStride: Int { return 32 }
internal static var breadcrumbStride: Int { 64 }

var utf16Length: Int
internal var utf16Length: Int

// TODO: does this need to be a pair?.... Can we be smaller than Int?
var crumbs: [String.Index]
internal var crumbs: [String.Index]

// TODO: Does this need to be inout, unique, or how will we be enforcing
// atomicity?
init(_ str: String) {
internal init(_ str: String) {
let stride = _StringBreadcrumbs.breadcrumbStride

self.crumbs = []
Expand Down Expand Up @@ -62,7 +62,7 @@ internal final class _StringBreadcrumbs {
}

extension _StringBreadcrumbs {
var stride: Int {
internal var stride: Int {
@inline(__always) get { return _StringBreadcrumbs.breadcrumbStride }
}

Expand Down Expand Up @@ -108,37 +108,3 @@ extension _StringBreadcrumbs {
#endif // INTERNAL_CHECKS_ENABLED
}

extension _StringGuts {
@_effects(releasenone)
internal func getBreadcrumbsPtr() -> UnsafePointer<_StringBreadcrumbs> {
_internalInvariant(hasBreadcrumbs)

let mutPtr: UnsafeMutablePointer<_StringBreadcrumbs?>
if hasNativeStorage {
mutPtr = _object.nativeStorage._breadcrumbsAddress
} else {
mutPtr = UnsafeMutablePointer(
Builtin.addressof(&_object.sharedStorage._breadcrumbs))
}

if _slowPath(mutPtr.pointee == nil) {
populateBreadcrumbs(mutPtr)
}

_internalInvariant(mutPtr.pointee != nil)
// assuming optional class reference and class reference can alias
return UnsafeRawPointer(mutPtr).assumingMemoryBound(to: _StringBreadcrumbs.self)
}

@inline(never) // slow-path
@_effects(releasenone)
internal func populateBreadcrumbs(
_ mutPtr: UnsafeMutablePointer<_StringBreadcrumbs?>
) {
// Thread-safe compare-and-swap
let crumbs = _StringBreadcrumbs(String(self))
_stdlib_atomicInitializeARCRef(
object: UnsafeMutableRawPointer(mutPtr).assumingMemoryBound(to: Optional<AnyObject>.self),
desired: crumbs)
}
}
1 change: 1 addition & 0 deletions stdlib/public/core/StringBridge.swift
Original file line number Diff line number Diff line change
Expand Up @@ -462,6 +462,7 @@ extension String {
// and bridge that instead. Also avoids CF deleting any BOM that may be
// present
var copy = self
// TODO: small capacity minimum is lifted, just need to make native
copy._guts.grow(_SmallString.capacity + 1)
_internalInvariant(!copy._guts.isSmall)
return copy._bridgeToObjectiveCImpl()
Expand Down
2 changes: 1 addition & 1 deletion stdlib/public/core/StringCreate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ extension String {
) throws -> Int
) rethrows -> String {
let result = try __StringStorage.create(
uninitializedCapacity: capacity,
uninitializedCodeUnitCapacity: capacity,
initializingUncheckedUTF8With: initializer)

switch validateUTF8(result.codeUnits) {
Expand Down
4 changes: 3 additions & 1 deletion stdlib/public/core/StringGuts.swift
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,10 @@ extension _StringGuts {

internal var hasSharedStorage: Bool { return _object.hasSharedStorage }

// Whether this string has breadcrumbs
internal var hasBreadcrumbs: Bool {
return hasNativeStorage || hasSharedStorage
return hasSharedStorage
|| (hasNativeStorage && _object.nativeStorage.hasBreadcrumbs)
}
}

Expand Down
33 changes: 18 additions & 15 deletions stdlib/public/core/StringGutsRangeReplaceable.swift
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ extension _StringGuts {
internal init(_initialCapacity capacity: Int) {
self.init()
if _slowPath(capacity > _SmallString.capacity) {
self.grow(capacity)
self.grow(capacity) // TODO: no factor should be applied
}
}

Expand All @@ -68,7 +68,7 @@ extension _StringGuts {
if let currentCap = self.uniqueNativeCapacity, currentCap >= n { return }

// Grow
self.grow(n)
self.grow(n) // TODO: no factor should be applied
}

// Grow to accomodate at least `n` code units
Expand All @@ -79,13 +79,16 @@ extension _StringGuts {
_internalInvariant(
self.uniqueNativeCapacity == nil || self.uniqueNativeCapacity! < n)

// TODO: Dont' do this! Growth should only happen for append...
let growthTarget = Swift.max(n, (self.uniqueNativeCapacity ?? 0) * 2)

if _fastPath(isFastUTF8) {
let isASCII = self.isASCII
let storage = self.withFastUTF8 {
__StringStorage.create(
initializingFrom: $0, capacity: growthTarget, isASCII: isASCII)
initializingFrom: $0,
codeUnitCapacity: growthTarget,
isASCII: isASCII)
}

self = _StringGuts(storage)
Expand Down Expand Up @@ -128,11 +131,11 @@ extension _StringGuts {
} else {
sufficientCapacity = false
}

if self.isUniqueNative && sufficientCapacity {
return
}

// If we have to resize anyway, and we fit in smol, we should have made one
_internalInvariant(totalCount > _SmallString.capacity)

Expand All @@ -145,7 +148,7 @@ extension _StringGuts {
growthTarget = Swift.max(
totalCount, _growArrayCapacity(nativeCapacity ?? 0))
}
self.grow(growthTarget)
self.grow(growthTarget) // NOTE: this already has exponential growth...
}

internal mutating func append(_ other: _StringGuts) {
Expand All @@ -157,7 +160,7 @@ extension _StringGuts {
}
append(_StringGutsSlice(other))
}

@inline(never)
@_effects(readonly)
private func _foreignConvertedToSmall() -> _SmallString {
Expand All @@ -170,7 +173,7 @@ extension _StringGuts {
_internalInvariant(smol._guts.isSmall)
return smol._guts.asSmall
}

private func _convertedToSmall() -> _SmallString {
_internalInvariant(utf8Count <= _SmallString.capacity)
if _fastPath(isSmall) {
Expand All @@ -186,25 +189,25 @@ extension _StringGuts {
defer { self._invariantCheck() }

let otherCount = slicedOther.utf8Count

let totalCount = utf8Count + otherCount

/*
Goal: determine if we need to allocate new native capacity
Possible scenarios in which we need to allocate:
• Not uniquely owned and native: we can't use the capacity to grow into,
have to become unique + native by allocating
• Not enough capacity: have to allocate to grow

Special case: a non-smol String that can fit in a smol String but doesn't
meet the above criteria shouldn't throw away its buffer just to be smol.
The reasoning here is that it may be bridged or have reserveCapacity'd
in preparation for appending more later, in which case we would end up
have to allocate anyway to convert back from smol.

If we would have to re-allocate anyway then that's not a problem and we
should just be smol.

e.g. consider
var str = "" // smol
str.reserveCapacity(100) // large native unique
Expand All @@ -215,15 +218,15 @@ extension _StringGuts {
nativeUnusedCapacity! >= otherCount
let shouldBeSmol = totalCount <= _SmallString.capacity &&
(isSmall || !hasEnoughUsableSpace)

if shouldBeSmol {
let smolSelf = _convertedToSmall()
let smolOther = String(Substring(slicedOther))._guts._convertedToSmall()
// TODO: In-register slicing
self = _StringGuts(_SmallString(smolSelf, appending: smolOther)!)
return
}

prepareForAppendInPlace(totalCount: totalCount, otherUTF8Count: otherCount)

if slicedOther.isFastUTF8 {
Expand Down
Loading

0 comments on commit 79bac4e

Please sign in to comment.