Skip to content
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

Jump to bottom

Refactors internal func _applyMapping using _FixedArray16 #31333

Merged
merged 1 commit into from Jun 4, 2020
Merged

Refactors internal func _applyMapping using _FixedArray16 #31333

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
42 changes: 32 additions & 10 deletions stdlib/public/core/UnicodeScalarProperties.swift
View file
Open in desktop
Expand Up @@ -692,29 +692,51 @@ extension Unicode.Scalar.Properties {
/// all current case mappings. In the event more space is needed, it will be
/// allocated on the heap.
Copy link
Collaborator

@xwu xwu May 24, 2020 •
edited

Choose a reason for hiding this comment

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

This doc comment doesn't match the implementation proposed in this PR.

IIUC, a buffer capable of holding 64 code units is allocated on the stack with this change instead of the heap, whereas as originally intended (but not implemented) only 16 code units would have been allocated on the stack, falling back to a larger heap allocation if needed.

I think perhaps instead what the original author had in mind to complete the TODO was as follows (typed freehand; check wellformedness with compiler):

internal func _applyMapping(_ u_strTo: _U_StrToX) -> String {
  // Allocate 16 code units on the stack.
  var fixedArray = _FixedArray16<UInt16>(allZeros: ())
  let count: Int = fixedArray.withUnsafeMutableBufferPointer { bufPtr in
    return _scalar.withUTF16CodeUnits { utf16 in
      var err = __swift_stdlib_U_ZERO_ERROR
      let correctSize = u_strTo(
        bufPtr.baseAddress._unsafelyUnwrappedUnchecked,
        Int32(bufPtr.count),
        utf16.baseAddress._unsafelyUnwrappedUnchecked,
        Int32(utf16.count),
        "",
        &err)
      guard err.isSuccess else {
        fatalError("Unexpected error case-converting Unicode scalar.")
      }
      return Int(correctSize)
    }
  }
  if _fastPath(count <= 16) {
    fixedArray.count = count
    return fixedArray.withUnsafeBufferPointer {
      return String._uncheckedFromUTF16($0)
    }
  }
  // Allocate `count` code units on the heap.
  var array = Array<UInt16>(repeating: 0, count: count)
  array.withUnsafeMutableBufferPointer { bufPtr in
    _scalar.withUTF16CodeUnits { utf16 in
      var err = __swift_stdlib_U_ZERO_ERROR
      let correctSize = u_strTo(
        bufPtr.baseAddress._unsafelyUnwrappedUnchecked,
        Int32(bufPtr.count),
        utf16.baseAddress._unsafelyUnwrappedUnchecked,
        Int32(utf16.count),
        "",
        &err)
      guard err.isSuccess else {
        fatalError("Unexpected error case-converting Unicode scalar.")
      }
      _internalInvariant(count == correctSize, "inconsistent ICU behavior")
    }
  }
  return array.withUnsafeBufferPointer {
    return String._uncheckedFromUTF16($0)
  }
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That absolutely makes sense. I've changed a little the second part.

internal func _applyMapping(_ u_strTo: _U_StrToX) -> String {
// TODO(String performance): Stack buffer first and then detect real count
let count = 64
var array = Array<UInt16>(repeating: 0, count: count)
let len: Int = array.withUnsafeMutableBufferPointer { bufPtr in
// Allocate 16 code units on the stack.
var fixedArray = _FixedArray16<UInt16>(allZeros: ())
let count: Int = fixedArray.withUnsafeMutableBufferPointer { buf in
return _scalar.withUTF16CodeUnits { utf16 in
var err = __swift_stdlib_U_ZERO_ERROR
let correctSize = u_strTo(
bufPtr.baseAddress._unsafelyUnwrappedUnchecked,
Int32(bufPtr.count),
buf.baseAddress._unsafelyUnwrappedUnchecked,
Int32(buf.count),
utf16.baseAddress._unsafelyUnwrappedUnchecked,
Int32(utf16.count),
"",
&err)
guard err.isSuccess else {
fatalError("Unexpected error case-converting Unicode scalar.")
}
// TODO: _internalInvariant(count == correctSize, "inconsistent ICU behavior")
return Int(correctSize)
}
}
// TODO: replace `len` with `count`
return array[..<len].withUnsafeBufferPointer {
return String._uncheckedFromUTF16($0)
if _fastPath(count <= 16) {
fixedArray.count = count
return fixedArray.withUnsafeBufferPointer {
String._uncheckedFromUTF16($0)
}
}
// Allocate `count` code units on the heap.
let array = Array<UInt16>(unsafeUninitializedCapacity: count) {
buf, initializedCount in
_scalar.withUTF16CodeUnits { utf16 in
var err = __swift_stdlib_U_ZERO_ERROR
let correctSize = u_strTo(
buf.baseAddress._unsafelyUnwrappedUnchecked,
Int32(buf.count),
utf16.baseAddress._unsafelyUnwrappedUnchecked,
Int32(utf16.count),
"",
&err)
guard err.isSuccess else {
fatalError("Unexpected error case-converting Unicode scalar.")
}
_internalInvariant(count == correctSize, "inconsistent ICU behavior")
initializedCount = count
}
}
return array.withUnsafeBufferPointer {
String._uncheckedFromUTF16($0)
}
}

Expand Down