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

Array<UInt8> is significantly slower than Data #14

Open
gormster opened this issue Jul 12, 2017 · 2 comments
Open

Array<UInt8> is significantly slower than Data #14

gormster opened this issue Jul 12, 2017 · 2 comments
Assignees
Milestone

Comments

@gormster
Copy link

While trying to debug some performance issues I was having today, I realised the root of my problems was an insanely slow parse in Multipart. Not that the work I was doing was too slow - the parser itself was slow.

I opened up the project in Xcode and added a simple test:

measure {
    let parser = try! Parser(boundary: "MimeBoundary_8923gr836bidfb3i")
    
    var parts: [Part] = []
    parser.onPart = { part in
        parts.append(part)
    }

    parts = []
    try! parser.parse(message)
    XCTAssertEqual(parts.count, 2)
}

For a 332kB message, with two parts (some text and a JPEG image), the implementation using Bytes (i.e. [UInt8]) took 0.422s. Replacing Bytes with Data resulted in the same test executing in 0.0475s. That's nearly a 10x speedup.

Now admittedly Multipart is a small library, and it wasn't zero work to change it over to use Data. Vapor as a whole is enormous, and such a changeover would require care and attention and a lot of work. But I think Bits really can't continue defining its own type for data when that type is so much slower than the native type. IMO, Bits should be a library for operating on Data objects, and Collections/Sequences of UInt8, and should ditch the Bytes type entirely.

@tanner0101
Copy link
Contributor

Two questions:

  • Was this compiled in release mode?
  • What operating system did you run these tests on (i'm assuming macOS)?

Release mode makes a huge difference when working with Bytes. Additionally, we've historically had huge performance issues using Data on Linux (however those may be fixed now).

I definitely want Vapor to move to Data eventually, we've just been running into a lot of issues with it. Hopefully some of those are starting to be solved. :)

@tanner0101 tanner0101 added this to the 3.0 milestone Jul 19, 2017
@tanner0101 tanner0101 self-assigned this Jul 19, 2017
@tanner0101 tanner0101 modified the milestones: 3.0, 2.0 Jul 19, 2017
@tanner0101
Copy link
Contributor

Marking this as 2.0 milestone since moving from Bytes ([UINT8]) -> Data would be a breaking change.

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

No branches or pull requests

2 participants