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

Add fast-path ByteBuffer.write method that takes a ByteBufferView. #517

Closed
Lukasa opened this issue Jul 16, 2018 · 3 comments
Closed

Add fast-path ByteBuffer.write method that takes a ByteBufferView. #517

Lukasa opened this issue Jul 16, 2018 · 3 comments
Labels
good first issue Good for newcomers kind/enhancement Improvements to existing feature.

Comments

@Lukasa
Copy link
Contributor

Lukasa commented Jul 16, 2018

Currently if you want to write a ByteBufferView to a ByteBuffer, it will fall into this method:

public mutating func write<S: Sequence>(bytes: S) -> Int where S.Element == UInt8

This ends up dropping into the very complex mutating func _set<S: Sequence>(bytes: S, at index: _Index) -> _Capacity where S.Element == UInt8, which is a very complex method that potentially performs multiple resizings and many bounds checks.

This is all unnecessary, as ByteBufferView is backed by a ByteBuffer. This means that we can instead call write(buffer:) with the appropriate details, and use the fast-path that that method uses to treat the collection as a ContiguousCollection.

Note that we don't need to (or want to) actually conform the ByteBufferView to ContiguousCollection. Instead we just want the same magic we've done with ByteBuffer to apply here: take our special knowledge of the layout of this type and use it to obtain a performance improvement.

@Lukasa Lukasa added kind/enhancement Improvements to existing feature. good first issue Good for newcomers labels Jul 16, 2018
@Lukasa
Copy link
Contributor Author

Lukasa commented Jul 16, 2018

This would be useful for apple/swift-nio-http2#10, which uses ByteBufferViews heavily and tends to copy them around.

@AlanQuatermain
Copy link
Contributor

Er. Isn't ByteBufferView already a ContiguousCollection…? The code certainly seems to indicate that:

public struct ByteBufferView: ContiguousCollection, RandomAccessCollection {

@Lukasa
Copy link
Contributor Author

Lukasa commented Jul 17, 2018

Hah, yup, sure is. Ignore me then!

@Lukasa Lukasa closed this as completed Jul 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers kind/enhancement Improvements to existing feature.
Projects
None yet
Development

No branches or pull requests

2 participants