Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

Embeddings: eliminate bounds checks and unroll#51006

Merged
camdencheek merged 3 commits into
cc/int8-embeddings-2from
cc/bounds-check-elimination
Apr 21, 2023
Merged

Embeddings: eliminate bounds checks and unroll#51006
camdencheek merged 3 commits into
cc/int8-embeddings-2from
cc/bounds-check-elimination

Conversation

@camdencheek

@camdencheek camdencheek commented Apr 21, 2023 •
edited
Loading

Copy link
Copy Markdown
Member

This checks the lengths of the input slices ahead of time so the compiler doesn't need to insert panics on slice accesses.

Bounds check elimination:

name                               old time/op    new time/op    delta
SimilaritySearch/numWorkers=1-10      559ms ± 0%     499ms ± 0%  -10.74%  (p=0.016 n=4+5)
SimilaritySearch/numWorkers=2-10      283ms ± 0%     253ms ± 0%  -10.65%  (p=0.008 n=5+5)
SimilaritySearch/numWorkers=4-10      146ms ± 0%     130ms ± 0%  -10.52%  (p=0.008 n=5+5)
SimilaritySearch/numWorkers=8-10     73.7ms ± 0%    66.0ms ± 0%  -10.45%  (p=0.008 n=5+5)
SimilaritySearch/numWorkers=16-10    83.6ms ± 2%    75.0ms ± 3%  -10.29%  (p=0.008 n=5+5)

Loop unrolling:

name                               old time/op    new time/op    delta
SimilaritySearch/numWorkers=1-10      499ms ± 0%     439ms ± 0%  -12.12%  (p=0.008 n=5+5)
SimilaritySearch/numWorkers=2-10      253ms ± 0%     222ms ± 0%  -12.21%  (p=0.008 n=5+5)
SimilaritySearch/numWorkers=4-10      130ms ± 0%     115ms ± 0%  -11.99%  (p=0.008 n=5+5)
SimilaritySearch/numWorkers=8-10     66.0ms ± 0%    57.9ms ± 0%  -12.20%  (p=0.008 n=5+5)
SimilaritySearch/numWorkers=16-10    75.0ms ± 3%    65.7ms ± 2%  -12.36%  (p=0.008 n=5+5)

Both:

name                               old time/op    new time/op    delta
SimilaritySearch/numWorkers=1-10      559ms ± 0%     439ms ± 0%  -21.55%  (p=0.016 n=4+5)
SimilaritySearch/numWorkers=2-10      283ms ± 0%     222ms ± 0%  -21.56%  (p=0.008 n=5+5)
SimilaritySearch/numWorkers=4-10      146ms ± 0%     115ms ± 0%  -21.24%  (p=0.008 n=5+5)
SimilaritySearch/numWorkers=8-10     73.7ms ± 0%    57.9ms ± 0%  -21.37%  (p=0.008 n=5+5)
SimilaritySearch/numWorkers=16-10    83.6ms ± 2%    65.7ms ± 2%  -21.38%  (p=0.008 n=5+5)

Test plan

Included benchmark results in PR. Added a quicktest to validate that, for all inputs, CosineSimilarity behaves the same as the naive implementation.

@cla-bot cla-bot Bot added the cla-signed label Apr 21, 2023
@camdencheek camdencheek force-pushed the cc/bounds-check-elimination branch from 72b3deb to 01a2da2 Compare April 21, 2023 19:02
@camdencheek camdencheek requested a review from a team April 21, 2023 19:06
@camdencheek camdencheek force-pushed the cc/bounds-check-elimination branch 3 times, most recently from 93fcd01 to 6f0344d Compare April 21, 2023 19:45
@camdencheek camdencheek force-pushed the cc/int8-embeddings-2 branch from 709f6fc to 61a3268 Compare April 21, 2023 19:46
@camdencheek camdencheek changed the title Embeddings: eliminate bounds checks Embeddings: eliminate bounds checks and unroll Apr 21, 2023
jtibshirani
jtibshirani approved these changes Apr 21, 2023
View reviewed changes

@jtibshirani jtibshirani left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice 😎

@camdencheek camdencheek force-pushed the cc/int8-embeddings-2 branch from 61a3268 to ad6a75f Compare April 21, 2023 21:57
@camdencheek camdencheek force-pushed the cc/bounds-check-elimination branch from d23d767 to 19ccca6 Compare April 21, 2023 21:58
@camdencheek camdencheek force-pushed the cc/bounds-check-elimination branch from 19ccca6 to 2e8d653 Compare April 21, 2023 21:58
@camdencheek camdencheek merged commit 63a408d into cc/int8-embeddings-2 Apr 21, 2023
@camdencheek camdencheek deleted the cc/bounds-check-elimination branch April 21, 2023 21:59
@jtibshirani jtibshirani mentioned this pull request Apr 22, 2023
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@jtibshirani jtibshirani jtibshirani approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cla-signed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@camdencheek @jtibshirani