All filter optimization #7300
All filter optimization #7300
Conversation
Signed-off-by: Calum Murray <[email protected]>
Signed-off-by: Calum Murray <[email protected]>
knative-prow
bot
added
the
size/M
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #7300 +/- ##
=======================================
Coverage 77.64% 77.65%
=======================================
Files 250 250
Lines 13436 13473 +37
=======================================
+ Hits 10432 10462 +30
- Misses 2480 2487 +7
Partials 524 524
☔ View full report in Codecov by Sentry. |
| } | ||
| } | ||
|
|
||
| func (filter *allFilter) optimize(swapIdx int) { |
There was a problem hiding this comment.
not sure i like the generic optimize func name...
can we add a bit why it does the swap?
| c chan int | ||
| d chan bool |
There was a problem hiding this comment.
More auto-describing names than c or d?
| // Short circuit to optimize it | ||
| if res == eventfilter.FailFilter { | ||
| filter.c <- i |
There was a problem hiding this comment.
I'm worried about this could block if the background gorouting is not listening at that point, I think we need buffering (with size 1 seems fine) and avoid blocking when the buffer is full
Signed-off-by: Calum Murray <[email protected]>
knative-prow
bot
added
the
do-not-merge/hold
knative-prow
bot
added
lgtm
knative-prow
bot
removed
the
do-not-merge/hold
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Cali0707, matzew, pierDipi The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Fixes #7147
Proposed Changes
The results of this optimization are:
However, it is worth noting that the "Creation" time actually measures creation and deletion time because I needed to clean up the goroutines.
Pre-review Checklist
Release Note