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

implement order preserving apis #12

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

implement order preserving apis #12

wants to merge 1 commit into from

Conversation

schmichael
Copy link
Member

Inspired by #11 but altered for a few reasons:

  1. I didn't want to change the API
  2. When deduplicating keys fix order issue for env expand #11 chose to keep the first position the key occurred at while I chose to keep the last so that the position of the key corresponds to the position of its value.
  3. I added some benchmarks to ensure the technically quadratic ordered implementation wasn't pathologically slower than the linear implementation.

The benchmarks show the ordered implementation to be faster for my synthesized "little" and "big" datasets:

BenchmarkLittle/Parse-20                  284787              3960 ns/op
BenchmarkLittle/ParsePairs-20             320619              3555 ns/op
BenchmarkBig/Parse-20                          1        1996228538 ns/op
BenchmarkBig/ParsePairs-20                     1        1900769619 ns/op

Inspired by #11 but altered for a few reasons:

1. I didn't want to change the API
2. When deduplicating keys #11 chose to keep the first position the key
   occurred at while I chose to keep the last so that the position of
   the key corresponds to the position of its value.
3. I added some benchmarks to ensure the technically quadratic ordered
   implementation wasn't pathologically slower than the linear
   implementation.

The benchmarks show the ordered implementation to be faster for my
synthesized "little" and "big" datasets:

```
BenchmarkLittle/Parse-20                  284787              3960 ns/op
BenchmarkLittle/ParsePairs-20             320619              3555 ns/op
BenchmarkBig/Parse-20                          1        1996228538 ns/op
BenchmarkBig/ParsePairs-20                     1        1900769619 ns/op
```
Copy link

@ysmood ysmood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

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

Successfully merging this pull request may close these issues.

None yet

2 participants