[c++-interop] Improve performance of creating a C++ std::string from a contiguous UTF-8 Swift.String

[CrazyFanFan]

Improve performance of creating a C++ std::string from a contiguous UTF-8 Swift.String #75110

[egorzhdan]

@swift-ci please smoke test

[CrazyFanFan]

linux compilation failed and I am trying to create a Linux virtual machine to debug the problem

[egorzhdan]

@swift-ci please smoke test

[egorzhdan]

@swift-ci please benchmark

[CrazyFanFan]

Hi @egorzhdan

I've made some updates to this PR;

1. Fixed the Linux compilation failure.
2. As expected, benchmark results demonstrate a significant efficiency improvement in initializing std::string from Swift.String.

---

------- Performance (x86_64): -O -------

IMPROVEMENT	OLD	NEW	DELTA	RATIO
CxxStringConversion.swift.to.cxx	98.136	0.476	-99.5%	205.74x

---

------- Performance (x86_64): -Osize -------

IMPROVEMENT	OLD	NEW	DELTA	RATIO
CxxStringConversion.swift.to.cxx	82.0	0.48	-99.4%	170.48x

---

------- Performance (x86_64): -Onone -------

IMPROVEMENT	OLD	NEW	DELTA	RATIO
CxxStringConversion.swift.to.cxx	81.864	0.504	-99.4%	162.11x

Could you please review it again when convenient?

Thanks

[egorzhdan]

This looks like a massive performance improvement!
I have one question.

[egorzhdan]

Is this check necessary? I think withCString should handle the non-contiguous case under the hood by falling back to the slow path of copying the string, which should produce similar results to the else branch here.

So I wonder if we could just remove the else branch in this initializer.

[egorzhdan]

@swift-ci please test

[egorzhdan]

@swift-ci please benchmark

[egorzhdan]

The relevant bits of the benchmark:

Performance (x86_64): -O

Improvement	OLD	NEW	DELTA	RATIO
CxxStringConversion.swift.to.cxx	98.143	0.488	-99.5%	200.70x

Code size: -O

Performance (x86_64): -Osize

Improvement	OLD	NEW	DELTA	RATIO
CxxStringConversion.swift.to.cxx	81.96	0.479	-99.4%	170.75x

Code size: -Osize

Performance (x86_64): -Onone

Improvement	OLD	NEW	DELTA	RATIO
CxxStringConversion.swift.to.cxx	82.077	0.5	-99.4%	163.83x

Code size: -swiftlibs

[egorzhdan]

Thanks, this is a significant performance improvement!

[compnerd]

I think that this might have caused a regression on Windows :(

D:/a/swift-build/swift-build/SourceCache/swift/stdlib/public/Cxx/std/String.swift:24:18: error: cannot convert value of type 'UnsafePointer<Unicode.UTF8.CodeUnit>' (aka 'UnsafePointer<UInt8>') to expected argument type 'std.basic_string<CChar, char_traits<CChar>, allocator<CChar>>'

[egorzhdan]

That is strange, the Windows CI was passing on this patch.
@compnerd is this failing in post-commit CI? If this is failing for you locally, could you please share the full error message?

[compnerd]

D:/a/swift-build/swift-build/SourceCache/swift/stdlib/public/Cxx/std/String.swift:24:18: error: cannot convert value of type 'UnsafePointer<Unicode.UTF8.CodeUnit>' (aka 'UnsafePointer<UInt8>') to expected argument type 'std.basic_string<CChar, char_traits<CChar>, allocator<CChar>>'
 22 |   public init(_ string: String) {
 23 |     self = string.withCString(encodedAs: UTF8.self) { buffer in
 24 |       std.string(buffer, string.utf8.count, .init())
    |                  `- error: cannot convert value of type 'UnsafePointer<Unicode.UTF8.CodeUnit>' (aka 'UnsafePointer<UInt8>') to expected argument type 'std.basic_string<CChar, char_traits<CChar>, allocator<CChar>>'
 25 |     }
 26 |   }

D:/a/swift-build/swift-build/SourceCache/swift/stdlib/public/Cxx/std/String.swift:30:17: error: cannot convert value of type 'UnsafePointer<CChar>' (aka 'UnsafePointer<Int8>') to expected argument type 'std.basic_string<CChar, char_traits<CChar>, allocator<CChar>>'
 28 |   public init(_ string: UnsafePointer<CChar>?) {
 29 |     if let str = string {
 30 |       self.init(str, UTF8._nullCodeUnitOffset(in: str), .init())
    |                 `- error: cannot convert value of type 'UnsafePointer<CChar>' (aka 'UnsafePointer<Int8>') to expected argument type 'std.basic_string<CChar, char_traits<CChar>, allocator<CChar>>'
 31 |     } else {
 32 |       self.init()

https://github.com/thebrowsercompany/swift-build/actions/runs/10456478071/job/28957616414#step:14:1611

[compnerd]

@egorzhdan I think that this is due to the Apple CI hosts being very out of date in the VC version.