Skip to content

Commit

Permalink
ByteBuffer: one fewer allocs to go to Data
Browse files Browse the repository at this point in the history
Motivation:

When getting a `Data` from a `ByteBuffer` we currently allocate twice
(`__DataStorage`) and the closure for `Data.Deallocator`.

Modifications:

We can optimise that by making the closure capture exactly one
`AnyObject` which is already a reference counted object. The compiler
realises that (on Linux) and saves us an alloc.

Thanks @Lukasa for the suggestion here: #1836 (comment)

Result:

Fewer allocs.
  • Loading branch information
weissi committed Apr 30, 2021
1 parent 6befe13 commit ada6eec
Show file tree
Hide file tree
Showing 8 changed files with 18 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ fi

"$here/../../allocation-counter-tests-framework/run-allocation-counter.sh" \
-p "$here/../../.." \
-m NIO -m NIOHTTP1 -m NIOWebSocket \
-m NIO -m NIOHTTP1 -m NIOWebSocket -m NIOFoundationCompat \
-s "$here/shared.swift" \
-t "$tmp_dir" \
"${tests_to_run[@]}"
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

import Dispatch
import NIO
import NIOFoundationCompat

func run(identifier: String) {
measure(identifier: identifier) {
Expand All @@ -25,37 +26,42 @@ func run(identifier: String) {
@inline(never)
func doWrites(buffer: inout ByteBuffer) {
/* these ones are zero allocations */
// buffer.writeBytes(foundationData) // see SR-7542
buffer.writeBytes([0x41])
buffer.writeBytes("A".utf8)
buffer.writeString("A")
buffer.writeStaticString("A")
buffer.writeInteger(0x41, as: UInt8.self)
buffer.writeRepeatingByte(0x41, count: 16)

/* those down here should be one allocation each (on Linux) */
buffer.writeBytes(dispatchData) // see https://bugs.swift.org/browse/SR-9597
}
@inline(never)
func doReads(buffer: inout ByteBuffer) {
/* these ones are zero allocations */
let val = buffer.readInteger(as: UInt8.self)
var val = buffer.readInteger(as: UInt8.self)
precondition(0x41 == val, "\(val!)")
val = buffer.readInteger(as: UInt16.self)
precondition(0x4141 == val, "\(val!)")
var slice = buffer.readSlice(length: 1)
let sliceVal = slice!.readInteger(as: UInt8.self)
precondition(0x41 == sliceVal, "\(sliceVal!)")
buffer.withUnsafeReadableBytes { ptr in
precondition(ptr[0] == 0x41)
}
let str = buffer.readString(length: 1)
precondition("A" == str, "\(str!)")

/* those down here should be one allocation each */
let arr = buffer.readBytes(length: 1)
precondition([0x41] == arr!, "\(arr!)")
let str = buffer.readString(length: 1)
precondition("A" == str, "\(str!)")
let data = buffer.readData(length: 16, byteTransferStrategy: .noCopy)
precondition(0x41 == data?.first && 0x41 == data?.last)
}
for _ in 0..<1000 {
doWrites(buffer: &buffer)
doReads(buffer: &buffer)
precondition(buffer.readableBytes == 0)
}
return buffer.readableBytes
}
Expand Down
4 changes: 2 additions & 2 deletions Sources/NIOFoundationCompat/ByteBuffer-foundation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -126,10 +126,10 @@ extension ByteBuffer {
return Data(bytes: UnsafeMutableRawPointer(mutating: ptr.baseAddress!.advanced(by: index)),
count: Int(length))
} else {
_ = storageRef.retain()
let storage = storageRef.takeUnretainedValue()
return Data(bytesNoCopy: UnsafeMutableRawPointer(mutating: ptr.baseAddress!.advanced(by: index)),
count: Int(length),
deallocator: .custom { _, _ in storageRef.release() })
deallocator: .custom { _, _ in withExtendedLifetime(storage) {} })
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion docker/docker-compose.1604.52.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ services:
- MAX_ALLOCS_ALLOWED_1000_udpbootstraps=2050
- MAX_ALLOCS_ALLOWED_1000_udpconnections=96050
- MAX_ALLOCS_ALLOWED_1_reqs_1000_conn=455000
- MAX_ALLOCS_ALLOWED_bytebuffer_lots_of_rw=2050
- MAX_ALLOCS_ALLOWED_bytebuffer_lots_of_rw=3050
- MAX_ALLOCS_ALLOWED_creating_10000_headers=0
- MAX_ALLOCS_ALLOWED_decode_1000_ws_frames=2050
- MAX_ALLOCS_ALLOWED_encode_1000_ws_frames_holding_buffer=3
Expand Down
2 changes: 1 addition & 1 deletion docker/docker-compose.1604.53.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ services:
- MAX_ALLOCS_ALLOWED_1000_udpbootstraps=2050
- MAX_ALLOCS_ALLOWED_1000_udpconnections=95050
- MAX_ALLOCS_ALLOWED_1_reqs_1000_conn=450050
- MAX_ALLOCS_ALLOWED_bytebuffer_lots_of_rw=2050
- MAX_ALLOCS_ALLOWED_bytebuffer_lots_of_rw=3050
- MAX_ALLOCS_ALLOWED_creating_10000_headers=0
- MAX_ALLOCS_ALLOWED_decode_1000_ws_frames=2050
- MAX_ALLOCS_ALLOWED_encode_1000_ws_frames_holding_buffer=3
Expand Down
2 changes: 1 addition & 1 deletion docker/docker-compose.1804.51.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ services:
- MAX_ALLOCS_ALLOWED_1000_udpbootstraps=2050
- MAX_ALLOCS_ALLOWED_1000_udpconnections=96050
- MAX_ALLOCS_ALLOWED_1_reqs_1000_conn=452050
- MAX_ALLOCS_ALLOWED_bytebuffer_lots_of_rw=2050
- MAX_ALLOCS_ALLOWED_bytebuffer_lots_of_rw=3050
- MAX_ALLOCS_ALLOWED_creating_10000_headers=10050
- MAX_ALLOCS_ALLOWED_decode_1000_ws_frames=2050
- MAX_ALLOCS_ALLOWED_encode_1000_ws_frames_holding_buffer=3
Expand Down
2 changes: 1 addition & 1 deletion docker/docker-compose.2004.54.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ services:
- MAX_ALLOCS_ALLOWED_1000_udpbootstraps=2050
- MAX_ALLOCS_ALLOWED_1000_udpconnections=96050
- MAX_ALLOCS_ALLOWED_1_reqs_1000_conn=453050 # regression from 5.3 which was 450050
- MAX_ALLOCS_ALLOWED_bytebuffer_lots_of_rw=2050
- MAX_ALLOCS_ALLOWED_bytebuffer_lots_of_rw=3050
- MAX_ALLOCS_ALLOWED_creating_10000_headers=0
- MAX_ALLOCS_ALLOWED_decode_1000_ws_frames=2050
- MAX_ALLOCS_ALLOWED_encode_1000_ws_frames_holding_buffer=3
Expand Down
2 changes: 1 addition & 1 deletion docker/docker-compose.2004.main.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ services:
- MAX_ALLOCS_ALLOWED_1000_udpbootstraps=2050
- MAX_ALLOCS_ALLOWED_1000_udpconnections=96050
- MAX_ALLOCS_ALLOWED_1_reqs_1000_conn=453000
- MAX_ALLOCS_ALLOWED_bytebuffer_lots_of_rw=2050
- MAX_ALLOCS_ALLOWED_bytebuffer_lots_of_rw=3050
- MAX_ALLOCS_ALLOWED_creating_10000_headers=0
- MAX_ALLOCS_ALLOWED_decode_1000_ws_frames=2050
- MAX_ALLOCS_ALLOWED_encode_1000_ws_frames_holding_buffer=3
Expand Down

0 comments on commit ada6eec

Please # to comment.