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

Faster keywordize #506

Merged
merged 7 commits into from
Aug 27, 2024
Merged

Faster keywordize #506

merged 7 commits into from
Aug 27, 2024

Conversation

bsless
Copy link
Contributor

@bsless bsless commented Aug 30, 2021

Implement walk with protocols and keywordize keys with it.
Gives about 2x speedup

left to do: decide on an implementation for cljs

@bsless
Copy link
Contributor Author

bsless commented Aug 30, 2021

@ikitommi do you want to extend this solution to cljs or keep it just for clj?

@bsless bsless changed the title WIP faster keywordize Faster keywordize Sep 1, 2021
@bsless
Copy link
Contributor Author

bsless commented Sep 1, 2021

Decided to split the implementation off only for Clojure since cljs is built differently, don't even know how walk is implemented for it

@bsless
Copy link
Contributor Author

bsless commented Sep 7, 2021

@ikitommi are there any blocking items for a review? I'd be happy to clear them if so.

@bsless
Copy link
Contributor Author

bsless commented Aug 25, 2024

New and improved version, about 4x faster for input

{"int" "10", "ints" ["1" "2" "3"], "map" {:1 "1", "2" "2"}}

@bsless
Copy link
Contributor Author

bsless commented Aug 26, 2024

Open question - how can we be sure this does not introduce regressions for users who upgrade?

@bsless
Copy link
Contributor Author

bsless commented Aug 26, 2024

Added generative test to validate my assumption

@ikitommi ikitommi merged commit bffe360 into metosin:master Aug 27, 2024
7 checks passed
@ikitommi
Copy link
Member

Thanks!

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

Successfully merging this pull request may close these issues.

2 participants