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

Remove cookie_factory #64

Merged
merged 4 commits into from
Jul 5, 2024
Merged

Conversation

Dinnerbone
Copy link
Contributor

Note that the code is almost exactly 1:1 with what we were doing before, just manually - not through cookie_factory

That means the code is ugly and inefficient and spaghetti.

It was before, too, you just couldn't tell as easily :D

I intentionally am not cleaning the code up as part of this PR, to make the diff easier to check if any bugs crept in.

@CUB3D
Copy link
Collaborator

CUB3D commented Jul 5, 2024

LGTM (well except the byteorder dep, but thats a discussion for another time)

@Dinnerbone
Copy link
Contributor Author

Benchmark results, because I got curious:

cargo build (debug)

Code Mean [s] Min [s] Max [s] Relative
Master 35.360 ± 0.390 35.048 35.797 1.72 ± 0.02
This PR 20.587 ± 0.087 20.505 20.678 1.00

cargo build --release

Code Mean [s] Min [s] Max [s] Relative
Master 90.870 ± 0.407 90.456 91.270 3.80 ± 0.02
This PR 23.932 ± 0.010 23.921 23.941 1.00

@Dinnerbone
Copy link
Contributor Author

LGTM (well except the byteorder dep, but thats a discussion for another time)

It's what we used in ruffle and I figured easy enough to switch; but it looks like we're exclusively BE, so switching away from this should be trivial in a followup if we wish

@Dinnerbone Dinnerbone merged commit bfc949b into ruffle-rs:master Jul 5, 2024
1 check passed
@Dinnerbone Dinnerbone mentioned this pull request Jul 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants