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

Improve performance of Include and Exclude functions #8851

Merged
merged 3 commits into from
Sep 19, 2017

Conversation

stuartcarnie
Copy link
Contributor

@stuartcarnie stuartcarnie commented Sep 19, 2017

  • given values slices are already sorted, use a binary search to find range
  • use copy builtin, which uses highly optimized assembly
  • added associated tests for IntegerValues to verify correctness

Benchmark of Include function

benchmark                                            old ns/op     new ns/op     delta
BenchmarkIntegerValues_IncludeNone_1000-8            651           6.69          -98.97%
BenchmarkIntegerValues_IncludeMiddleHalf_1000-8      1131          114           -89.92%
BenchmarkIntegerValues_IncludeFirst_1000-8           638           33.9          -94.69%
BenchmarkIntegerValues_IncludeLast_1000-8            1269          32.2          -97.46%
BenchmarkIntegerValues_IncludeNone_10000-8           7751          6.76          -99.91%
BenchmarkIntegerValues_IncludeMiddleHalf_10000-8     11582         1378          -88.10%
BenchmarkIntegerValues_IncludeFirst_10000-8          7911          43.8          -99.45%
BenchmarkIntegerValues_IncludeLast_10000-8           12442         38.4          -99.69%

Benchmark of Exclude function

benchmark                                            old ns/op     new ns/op     delta
BenchmarkIntegerValues_ExcludeNone_1000-8            1285          7.34          -99.43%
BenchmarkIntegerValues_ExcludeMiddleHalf_1000-8      1258          148           -88.24%
BenchmarkIntegerValues_ExcludeFirst_1000-8           1268          7.51          -99.41%
BenchmarkIntegerValues_ExcludeLast_1000-8            1125          27.7          -97.54%
BenchmarkIntegerValues_ExcludeNone_10000-8           12665         7.31          -99.94%
BenchmarkIntegerValues_ExcludeMiddleHalf_10000-8     12039         976           -91.89%
BenchmarkIntegerValues_ExcludeFirst_10000-8          12663         7.29          -99.94%
BenchmarkIntegerValues_ExcludeLast_10000-8           10990         34.9          -99.68%
Required for all non-trivial PRs
  • Rebased/mergable
  • Tests pass
  • CHANGELOG.md updated

```
± benchcmp old.txt new.txt
benchmark                                            old ns/op     new ns/op     delta
BenchmarkIntegerValues_ExcludeNone_1000-8            1285          7.34          -99.43%
BenchmarkIntegerValues_ExcludeMiddleHalf_1000-8      1258          148           -88.24%
BenchmarkIntegerValues_ExcludeFirst_1000-8           1268          7.51          -99.41%
BenchmarkIntegerValues_ExcludeLast_1000-8            1125          27.7          -97.54%
BenchmarkIntegerValues_ExcludeNone_10000-8           12665         7.31          -99.94%
BenchmarkIntegerValues_ExcludeMiddleHalf_10000-8     12039         976           -91.89%
BenchmarkIntegerValues_ExcludeFirst_10000-8          12663         7.29          -99.94%
BenchmarkIntegerValues_ExcludeLast_10000-8           10990         34.9          -99.68%
```

(cherry picked from commit d7a3c23)
```
benchmark                                            old ns/op     new ns/op     delta
BenchmarkIntegerValues_IncludeNone_1000-8            651           6.69          -98.97%
BenchmarkIntegerValues_IncludeMiddleHalf_1000-8      1131          114           -89.92%
BenchmarkIntegerValues_IncludeFirst_1000-8           638           33.9          -94.69%
BenchmarkIntegerValues_IncludeLast_1000-8            1269          32.2          -97.46%
BenchmarkIntegerValues_IncludeNone_10000-8           7751          6.76          -99.91%
BenchmarkIntegerValues_IncludeMiddleHalf_10000-8     11582         1378          -88.10%
BenchmarkIntegerValues_IncludeFirst_10000-8          7911          43.8          -99.45%
BenchmarkIntegerValues_IncludeLast_10000-8           12442         38.4          -99.69%
```

(cherry picked from commit fb93ad5)
Copy link
Contributor

@jwilder jwilder left a comment

Choose a reason for hiding this comment

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

Nice speedup!

@stuartcarnie stuartcarnie merged commit 2ec4f55 into master Sep 19, 2017
@stuartcarnie stuartcarnie deleted the sgc-values-perf branch September 19, 2017 17:19
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