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

Added optional hash table indexmap #28

Merged
merged 4 commits into from Jul 20, 2023
Merged

Added optional hash table indexmap #28

merged 4 commits into from Jul 20, 2023

Conversation

ghost
Copy link

@ghost ghost commented Jul 19, 2023

First of all, thank you for this dependency and all your hard work on it.

Personally I use IndexMap instead of BTreeMap for my projects, so I think it would be nice to allow people to choose IndexMap as their preferred hash table for conversion.

An example of why I use IndexMap
indexmap-rs/indexmap#269

test a_new_btreemap       ... bench:           1 ns/iter (+/- 0)
test a_new_indexmap       ... bench:           3 ns/iter (+/- 0)

test b_insert_btreemap    ... bench:  60,621,040 ns/iter (+/- 15,248,541)
test b_insert_indexmap    ... bench:  42,826,040 ns/iter (+/- 1,214,166)

test c_clone_btreemap     ... bench:  41,720,370 ns/iter (+/- 1,932,831)
test c_clone_indexmap     ... bench:  39,500,870 ns/iter (+/- 1,026,400)

test d_for_each_btreemap  ... bench:   8,487,320 ns/iter (+/- 112,188)
test d_for_each_indexmap  ... bench:   7,427,390 ns/iter (+/- 177,412)

test e_find_btreemap      ... bench:          16 ns/iter (+/- 1)
test e_find_indexmap      ... bench:          13 ns/iter (+/- 0)

test f_rfind_btreemap     ... bench:          23 ns/iter (+/- 1)
test f_rfind_indexmap     ... bench:          14 ns/iter (+/- 0)

//Edit
I also updated all dependencies directly

@Diggsey
Copy link
Owner

Diggsey commented Jul 20, 2023

Hi @ThisAccountHasBeenSuspended - ijson always uses its own representation internally, this is just a conversion.

With that in mind, the new feature should not disable the existing conversion from BTreeMap - when the indexmap feature flag is enabled the new conversion should exist in addition to the old conversion.

src/value.rs Show resolved Hide resolved
@Diggsey
Copy link
Owner

Diggsey commented Jul 20, 2023

Thanks 👍

@Diggsey Diggsey merged commit e349c9a into Diggsey:master Jul 20, 2023
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.

1 participant