-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
feat: change the logic to handle dynamic batching #6066
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #6066 +/- ##
==========================================
+ Coverage 76.75% 76.87% +0.12%
==========================================
Files 145 145
Lines 13925 13986 +61
==========================================
+ Hits 10688 10752 +64
+ Misses 3237 3234 -3
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
Signed-off-by: Joan Fontanals Martinez <joan.martinez@jina.ai>
80445a0
to
1004f02
Compare
Signed-off-by: Joan Fontanals Martinez <joan.martinez@jina.ai>
6ff2cfe
to
d351bd7
Compare
Signed-off-by: Joan Fontanals Martinez <joan.martinez@jina.ai>
f0b5820
to
f7561ab
Compare
Signed-off-by: Joan Fontanals Martinez <joan.martinez@jina.ai>
19d1dcf
to
a6df665
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just one tiny comment
a6df665
to
3bf6e6e
Compare
Signed-off-by: Joan Fontanals Martinez <joan.martinez@jina.ai>
3bf6e6e
to
91a8a78
Compare
btw i think it is fair to call this a feature or at least a performance improvement, the old implementation was correct (we never made any guarantees that the actual batch size would not be bigger than the preferred one), just not optimized for this scenario. |
This PR fixes the behavior of dynamic batching.