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

Switch to user Foundation's Data instead of [UInt8] #9

Open
cloutiertyler opened this issue Oct 30, 2016 · 8 comments
Open

Switch to user Foundation's Data instead of [UInt8] #9

cloutiertyler opened this issue Oct 30, 2016 · 8 comments
Assignees

Comments

@cloutiertyler
Copy link
Collaborator

Rather than pass around [UInt8], it would be better to use Foundation's Data object which has a much more versatile API and can be readily converted to [UInt8] in any case.

@robertjpayne
Copy link

I wouldn't do this, There is a good reason Zewo/Perfect/Vapor are not using Foundation.Data because it's slow and inflexible for raw-memory access.

@cloutiertyler
Copy link
Collaborator Author

cloutiertyler commented Oct 30, 2016

So this is an interesting question actually. Here's the thing with [UInt8], there is a lot of magic going on with copy on write and sometimes it can be difficult to control or not well defined. It's also not so fun to deal with all the conversions.

Take a look at this: http://robnapier.net/nsdata

And a lot of the problems with NSData are now resolved with Data in Swift 3. For example, Data now conforms to ReferenceConvertible, Equatable, Hashable, RandomAccessCollection, MutableCollection, and RangeReplaceableCollection. There is a lot of reinventing the wheel going on here and here. I would wager that perhaps the main reason that they aren't using Data is that they created the frameworks before the API was sufficiently Swift-y.

In terms of performance, I must say I don't know all of the implications, but at least if we use the standard type we can ride the rising tide. At the end of the day, I think the Foundation team is be better equipped to handle low level data manipulations and transformations and they shouldn't be owned by a networking framework.

One other thing to consider is familiarity. All developers using (particularly iOS) are going to be familiar with the Data's APIs. I'd like to be careful not to get into business of adding a few convenience extensions here and there on [UInt8] and then end up with just another data manipulation API that you have to learn specifically for Edge, as is the case with the other frameworks currently.

I'd be interested to hear your thought though.

@cloutiertyler
Copy link
Collaborator Author

There's also the point that NSData can be backed by a dispatch_data_t which can have storage that is not contiguous. Since we are using libdispatch, that could potentially save copying and performance. The APIs also become simpler to use.

@robertjpayne
Copy link

@TheArtOfEngineering we tried both Data and DispatchData with Zewo we found but unsatisfactory for high performance.

Data's API for dealing with memory pointers is no better than [UInt8] and it performs copies sometimes when it doesn't need to.

DispatchData while seemingly great actually can never benefit from "stack optimisations" and thus slowed down things a lot.

In time hopefully some of these things are resolved…

@cloutiertyler
Copy link
Collaborator Author

@robertjpayne

I see, so you have been down this road. Thanks for the heads up on performance! I hope those issues will be resolved in the near future. Do you happen to know if Zewo published those performance tests somewhere? It would be an interesting read.

@robertjpayne
Copy link

@TheArtOfEngineering we didn't publish results and we didn't file any bugs with the Swift team because we didn't really feel they were bugs, just different tradeoffs.

In general DispatchData was half the performance of [UInt8] and Data wasn't too much better than DispatchData.

I can't remember the exact reasons for ditching Data and going with a custom wrap around [UInt8] but I believe it came down to inability to optimise when it copies and also that the Swift Compiler seems to make extra optimisations to [UInt8](my guess is stack inlining) where as Data is always operating on the heap.

DispatchData has similar issues in that you always are operating on the heap and while it's easier to optimise when it copies DispatchData is immutable so you sometimes have to copy to construct DispatchData.

@cloutiertyler
Copy link
Collaborator Author

@robertjpayne Alright, I'll have to hold off on this change then. I'll try to make some performance tests for this and we can wait and see if the performance improves with Data.

I thought I saw mentioned on the Swift mailing list that arrays were only heap allocated in Swift, but maybe they've added some optimizations.

@cloutiertyler
Copy link
Collaborator Author

@robertjpayne Do you know if the status of this changed at all? I noticed that libraries like https://github.com/apple/swift-protobuf are now using Data.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants