Added filter order optimizations by Cali0707 · Pull Request #3389 · knative-extensions/eventing-kafka-broker

Cali0707 · 2023-10-10T20:40:58Z

This PR brings the filter order optimizations from eventing core to the java filter implementation

Proposed Changes

Create a FilterListOptimizer class that handles the optimization loop
Use the FilterListOptimizer class with the AnyFilter and AllFilter

Release Note

The Any filter and All filter are now dynamically reordered for performance

Docs

Signed-off-by: Calum Murray <cmurray@redhat.com>

knative-prow · 2023-10-10T20:41:09Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Cali0707

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [Cali0707]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Cali0707 · 2023-10-10T20:41:41Z

/cc @pierDipi

codecov · 2023-10-10T20:47:16Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 58.34%. Comparing base (794302d) to head (afaffb0).
⚠️ Report is 631 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #3389      +/-   ##
============================================
- Coverage     61.48%   58.34%   -3.14%     
============================================
  Files           181       91      -90     
  Lines         12356     9279    -3077     
  Branches        265        0     -265     
============================================
- Hits           7597     5414    -2183     
+ Misses         4159     3431     -728     
+ Partials        600      434     -166

Flag	Coverage Δ
java-unittests	`?`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Cali0707 · 2023-10-11T02:13:52Z

/hold
Until I run benchmarks to confirm this is faster...

pierDipi · 2023-10-11T07:44:43Z

+import java.util.concurrent.locks.ReadWriteLock;
+import org.slf4j.Logger;
+
+public class FilterListOptimizer extends Thread {


A thread is way bigger than a go routing and during our scalability tests we reached the maximum thread count (which can't be easily increased in many platforms), so having more threads is a bit problematic until we have loom threads I don't really recommend increasing thread count (1 per consumer group is a lot)

I believe what we might gain from runtime optimizations we will lose in higher memory usage and risk of blocking the Vertx event loop

pierDipi · 2023-10-11T07:49:51Z

+    private final List<FilterCounter> filters;
+
+    private final ArrayBlockingQueue<Integer> indexSwapQueue;
+
+    private final FilterListOptimizer filterListOptimizer;
+
+    private final ReadWriteLock readWriteLock;


Generally, Vertx doesn't like locks and blocking operations, I'm pretty sure with this implementation we will block the event loop and that causes basically to block any event delivery for a particular trigger

Signed-off-by: Calum Murray <cmurray@redhat.com>

Cali0707 · 2023-10-13T20:26:58Z

@pierDipi @matzew not sure if either of you have experience with Vert.x + JMH, but I am getting the following error while running benchmarks and am not sure how to fix it:

<JMH had finished, but forked VM did not exit, are there stray running threads? Waiting 24 seconds more...>

Non-finished threads:

Thread[#33,DestroyJavaVM,5,main]

Thread[#32,vert.x-eventloop-thread-0,5,main]
  at java.base@20.0.2/sun.nio.ch.EPoll.wait(Native Method)
  at java.base@20.0.2/sun.nio.ch.EPollSelectorImpl.doSelect(EPollSelectorImpl.java:121)
  at java.base@20.0.2/sun.nio.ch.SelectorImpl.lockAndDoSelect(SelectorImpl.java:130)
  at java.base@20.0.2/sun.nio.ch.SelectorImpl.select(SelectorImpl.java:147)
  at app//io.netty.channel.nio.SelectedSelectionKeySetSelector.select(SelectedSelectionKeySetSelector.java:68)
  at app//io.netty.channel.nio.NioEventLoop.select(NioEventLoop.java:879)
  at app//io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:526)
  at app//io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:997)
  at app//io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74)
  at app//io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
  at java.base@20.0.2/java.lang.Thread.runWith(Thread.java:1636)
  at java.base@20.0.2/java.lang.Thread.run(Thread.java:1623)

Signed-off-by: Calum Murray <cmurray@redhat.com>

Cali0707 · 2023-10-18T19:12:30Z

/unhold
/cc @pierDipi @matzew @aliok @creydr
With these changes we see the following performance tradeoffs:

AnyFilter

 name                                                                                                         mode    old count   new count   units    diff        
 AnyFilterBenchmark.AnyFilter2EventsMatch2DifferentFilters.benchmarkFilterCreation                            thrpt   4.535       2.046       ops/us   -54.88423%  
 AnyFilterBenchmark.AnyFilter2EventsMatch2DifferentFilters.benchmarkFilterEvaluation                          thrpt   37.575      60.47       ops/us   +60.93147%  
 AnyFilterBenchmark.AnyFilter2EventsMatch2DifferentFiltersOneFilterMatchesNeither.benchmarkFilterCreation     thrpt   2.994       1.487       ops/us   -50.33400%  
 AnyFilterBenchmark.AnyFilter2EventsMatch2DifferentFiltersOneFilterMatchesNeither.benchmarkFilterEvaluation   thrpt   23.86       45.153      ops/us   +89.24141%  
 AnyFilterBenchmark.AnyFilterFirstMatchAtEnd.benchmarkFilterCreation                                          thrpt   2.977       1.506       ops/us   -49.41216%  
 AnyFilterBenchmark.AnyFilterFirstMatchAtEnd.benchmarkFilterEvaluation                                        thrpt   21.248      62.942      ops/us   +196.22553% 
 AnyFilterBenchmark.AnyFilterFirstMatchAtStart.benchmarkFilterCreation                                        thrpt   2.994       1.509       ops/us   -49.59920%  
 AnyFilterBenchmark.AnyFilterFirstMatchAtStart.benchmarkFilterEvaluation                                      thrpt   107.058     88.027      ops/us   -17.77635%  
 AnyFilterBenchmark.AnyFilterMatchAllSubfilters.benchmarkFilterCreation                                       thrpt   2.832       1.465       ops/us   -48.26977%  
 AnyFilterBenchmark.AnyFilterMatchAllSubfilters.benchmarkFilterEvaluation                                     thrpt   106.5       87.972      ops/us   -17.39718%  
 AnyFilterBenchmark.AnyFilterWithExactFilterBenchmark.benchmarkFilterCreation                                 thrpt   8.954       2.513       ops/us   -71.93433%  
 AnyFilterBenchmark.AnyFilterWithExactFilterBenchmark.benchmarkFilterEvaluation                               thrpt   122.93      91.047      ops/us   -25.93590%

AllFilter

 name                                                                                 mode    old count   new count   units    diff       
 AllFilterBenchmark.AllFilterFirstMatchEndOfArray.benchmarkFilterCreation             thrpt   2.698       1.518       ops/us   -43.73610% 
 AllFilterBenchmark.AllFilterFirstMatchEndOfArray.benchmarkFilterEvaluation           thrpt   69.308      68.486      ops/us   -1.18601%  
 AllFilterBenchmark.AllFilterFirstMatchStartOfArray.benchmarkFilterCreation           thrpt   2.746       1.461       ops/us   -46.79534% 
 AllFilterBenchmark.AllFilterFirstMatchStartOfArray.benchmarkFilterEvaluation         thrpt   42.821      51.506      ops/us   +20.28210% 
 AllFilterBenchmark.AllFilterMatchAllSubFilters.benchmarkFilterCreation               thrpt   2.797       1.42        ops/us   -49.23132% 
 AllFilterBenchmark.AllFilterMatchAllSubFilters.benchmarkFilterEvaluation             thrpt   17.866      18.442      ops/us   +3.22400%  
 AllFilterBenchmark.AllFilterNoMatchingFilters.benchmarkFilterCreation                thrpt   4.095       1.863       ops/us   -54.50549% 
 AllFilterBenchmark.AllFilterNoMatchingFilters.benchmarkFilterEvaluation              thrpt   69.715      68.882      ops/us   -1.19486%  
 AllFilterBenchmark.AllFilterOneNonMatchingFilterInMiddle.benchmarkFilterCreation     thrpt   2.649       1.502       ops/us   -43.29936% 
 AllFilterBenchmark.AllFilterOneNonMatchingFilterInMiddle.benchmarkFilterEvaluation   thrpt   39.46       47.646      ops/us   +20.74506% 
 AllFilterBenchmark.AllFilterWithExactFilter.benchmarkFilterCreation                  thrpt   7.963       2.515       ops/us   -68.41643% 
 AllFilterBenchmark.AllFilterWithExactFilter.benchmarkFilterEvaluation                thrpt   107.595     98.031      ops/us   -8.88889%

I think these are reasonable tradeoffs, but if not let me know and I'm happy to close this PR :)

Also, it is worth noting that the AllFilterBenchmarks don'tn test for a case when this re-ordering will lead to the largest speedup i.e. one non-matching filter at the end of the filters array, so we would see an even larger speedup there.

Cali0707 · 2023-11-15T13:20:01Z

/test reconciler-tests-keda

knative-prow · 2023-11-15T13:49:09Z

@Cali0707: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
channel-integration-tests-ssl_eventing-kafka-broker_main	`afaffb0`	link	true	`/test channel-integration-tests-ssl`
reconciler-tests-keda_eventing-kafka-broker_main	`afaffb0`	link	false	`/test reconciler-tests-keda`

Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

Cali0707 · 2023-12-19T16:18:39Z

Okay, with the benchmark logging issues fixed, I was able to benchmark this optimization and track the memory usage diff. The results were:

 name                                                                                                                       mode    old count   new count   units    diff
 AnyFilterBenchmark.AnyFilter2EventsMatch2DifferentFilters.benchmarkFilterCreation                                          thrpt   4.687       2.062       ops/us   -56.00597%
 AnyFilterBenchmark.AnyFilter2EventsMatch2DifferentFilters.benchmarkFilterCreation:gc.alloc.norm                            thrpt   1240.0      1608.07     B/op     +29.68306%
 AnyFilterBenchmark.AnyFilter2EventsMatch2DifferentFilters.benchmarkFilterEvaluation                                        thrpt   40.176      44.505      ops/us   +10.77509%
 AnyFilterBenchmark.AnyFilter2EventsMatch2DifferentFilters.benchmarkFilterEvaluation:gc.alloc.norm                          thrpt   0           0           B/op     +0.00000%
 AnyFilterBenchmark.AnyFilter2EventsMatch2DifferentFiltersOneFilterMatchesNeither.benchmarkFilterCreation                   thrpt   3.161       1.599       ops/us   -49.41474%
 AnyFilterBenchmark.AnyFilter2EventsMatch2DifferentFiltersOneFilterMatchesNeither.benchmarkFilterCreation:gc.alloc.norm     thrpt   1776.0      2152.179    B/op     +21.17562%
 AnyFilterBenchmark.AnyFilter2EventsMatch2DifferentFiltersOneFilterMatchesNeither.benchmarkFilterEvaluation                 thrpt   22.754      40.077      ops/us   +76.13167%
 AnyFilterBenchmark.AnyFilter2EventsMatch2DifferentFiltersOneFilterMatchesNeither.benchmarkFilterEvaluation:gc.alloc.norm   thrpt   0           0           B/op     +0.00000%
 AnyFilterBenchmark.AnyFilterFirstMatchAtEnd.benchmarkFilterCreation                                                        thrpt   3.139       1.565       ops/us   -50.14336%
 AnyFilterBenchmark.AnyFilterFirstMatchAtEnd.benchmarkFilterCreation:gc.alloc.norm                                          thrpt   1776.0      2120.075    B/op     +19.37359%
 AnyFilterBenchmark.AnyFilterFirstMatchAtEnd.benchmarkFilterEvaluation                                                      thrpt   24.342      59.753      ops/us   +145.47285%
 AnyFilterBenchmark.AnyFilterFirstMatchAtEnd.benchmarkFilterEvaluation:gc.alloc.norm                                        thrpt   0           0           B/op     +00.00000%
 AnyFilterBenchmark.AnyFilterFirstMatchAtStart.benchmarkFilterCreation                                                      thrpt   2.898       1.573       ops/us   -45.72119%
 AnyFilterBenchmark.AnyFilterFirstMatchAtStart.benchmarkFilterCreation:gc.alloc.norm                                        thrpt   1840.0      1992.083    B/op     +8.26538%
 AnyFilterBenchmark.AnyFilterFirstMatchAtStart.benchmarkFilterEvaluation                                                    thrpt   95.931      92.619      ops/us   -3.45248%
 AnyFilterBenchmark.AnyFilterFirstMatchAtStart.benchmarkFilterEvaluation:gc.alloc.norm                                      thrpt   0           0           B/op     +0.00000%
 AnyFilterBenchmark.AnyFilterMatchAllSubfilters.benchmarkFilterCreation                                                     thrpt   2.919       1.598       ops/us   -45.25522%
 AnyFilterBenchmark.AnyFilterMatchAllSubfilters.benchmarkFilterCreation:gc.alloc.norm                                       thrpt   1840.0      2248.075    B/op     +22.17798%
 AnyFilterBenchmark.AnyFilterMatchAllSubfilters.benchmarkFilterEvaluation                                                   thrpt   95.821      88.887      ops/us   -7.23641%
 AnyFilterBenchmark.AnyFilterMatchAllSubfilters.benchmarkFilterEvaluation:gc.alloc.norm                                     thrpt   0           0           B/op     +0.00000%
 AnyFilterBenchmark.AnyFilterWithExactFilterBenchmark.benchmarkFilterCreation                                               thrpt   9.04        2.954       ops/us   -67.32301%
 AnyFilterBenchmark.AnyFilterWithExactFilterBenchmark.benchmarkFilterCreation:gc.alloc.norm                                 thrpt   672.0       920.068     B/op     +36.91488%
 AnyFilterBenchmark.AnyFilterWithExactFilterBenchmark.benchmarkFilterEvaluation                                             thrpt   119.506     92.287      ops/us   -22.77626%
 AnyFilterBenchmark.AnyFilterWithExactFilterBenchmark.benchmarkFilterEvaluation:gc.alloc.norm                               thrpt   0           0           B/op     +0.00000%

So, on average this increased the memory usage of an any filter by approx. 400 bytes. For an all filter, this should be the same as the same code was used.

IMO, this is a reasonable tradeoff for the large (100%+) throughput improvements

github-actions · 2024-03-19T01:25:05Z

This Pull Request is stale because it has been open for 90 days with
no activity. It will automatically close after 30 more days of
inactivity. Reopen with /reopen. Mark as fresh by adding the
comment /remove-lifecycle stale.

github-actions · 2024-06-18T01:30:42Z

This Pull Request is stale because it has been open for 90 days with
no activity. It will automatically close after 30 more days of
inactivity. Reopen with /reopen. Mark as fresh by adding the
comment /remove-lifecycle stale.

Cali0707 · 2024-06-20T18:36:43Z

/remove-lifecycle stale
@pierDipi is this something we should work to include now or should we close it?

knative-prow-robot · 2024-07-27T15:20:15Z

PR needs rebase.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

github-actions · 2024-12-02T01:33:18Z

This Pull Request is stale because it has been open for 90 days with
no activity. It will automatically close after 30 more days of
inactivity. Reopen with /reopen. Mark as fresh by adding the
comment /remove-lifecycle stale.

github-actions · 2025-03-04T01:32:53Z

This Pull Request is stale because it has been open for 90 days with
no activity. It will automatically close after 30 more days of
inactivity. Reopen with /reopen. Mark as fresh by adding the
comment /remove-lifecycle stale.

github-actions · 2025-06-03T01:33:52Z

This Pull Request is stale because it has been open for 90 days with
no activity. It will automatically close after 30 more days of
inactivity. Reopen with /reopen. Mark as fresh by adding the
comment /remove-lifecycle stale.

github-actions · 2025-09-03T01:32:37Z

This Pull Request is stale because it has been open for 90 days with
no activity. It will automatically close after 30 more days of
inactivity. Reopen with /reopen. Mark as fresh by adding the
comment /remove-lifecycle stale.

github-actions · 2025-12-03T01:34:02Z

This Pull Request is stale because it has been open for 90 days with
no activity. It will automatically close after 30 more days of
inactivity. Reopen with /reopen. Mark as fresh by adding the
comment /remove-lifecycle stale.

github-actions · 2026-03-04T01:36:51Z

This Pull Request is stale because it has been open for 90 days with
no activity. It will automatically close after 30 more days of
inactivity. Reopen with /reopen. Mark as fresh by adding the
comment /remove-lifecycle stale.

Added filter order optimizations

38088f7

Signed-off-by: Calum Murray <cmurray@redhat.com>

knative-prow Bot requested review from matzew and odacremolbap October 10, 2023 20:41

knative-prow Bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Oct 10, 2023

knative-prow Bot added approved Indicates a PR has been approved by an approver from all required OWNERS files. area/data-plane labels Oct 10, 2023

knative-prow Bot requested a review from pierDipi October 10, 2023 20:41

knative-prow Bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 11, 2023

pierDipi reviewed Oct 11, 2023

View reviewed changes

Cali0707 added 2 commits October 13, 2023 16:08

Made filter optimization with vertx periodic instead of threads

6a4af89

Signed-off-by: Calum Murray <cmurray@redhat.com>

Small benchmark fixes

3958150

Signed-off-by: Calum Murray <cmurray@redhat.com>

Cali0707 requested a review from pierDipi October 13, 2023 20:27

Cali0707 added 2 commits October 16, 2023 11:02

fixed benchmarks

a4792a8

Signed-off-by: Calum Murray <cmurray@redhat.com>

further optimizations to base loop performance

536a774

Signed-off-by: Calum Murray <cmurray@redhat.com>

knative-prow-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 17, 2023

Cali0707 added 4 commits October 18, 2023 12:00

further improvements

573874c

Signed-off-by: Calum Murray <cmurray@redhat.com>

Speed up filter creation time

0fbd558

Signed-off-by: Calum Murray <cmurray@redhat.com>

Final fixes

35bc707

Signed-off-by: Calum Murray <cmurray@redhat.com>

fixed merge conflicts

afaffb0

Signed-off-by: Calum Murray <cmurray@redhat.com>

knative-prow-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 18, 2023

knative-prow Bot requested review from aliok and creydr October 18, 2023 19:12

knative-prow Bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 18, 2023

github-actions Bot added lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Mar 19, 2024

github-actions Bot added lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jun 18, 2024

knative-prow-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 27, 2024

github-actions Bot added lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Dec 2, 2024

github-actions Bot added lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Mar 4, 2025

github-actions Bot added lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jun 3, 2025

github-actions Bot added lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Sep 3, 2025

github-actions Bot added lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Dec 3, 2025

github-actions Bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Mar 4, 2026

github-actions Bot closed this Apr 3, 2026

Conversation

Cali0707 commented Oct 10, 2023

Proposed Changes

Uh oh!

knative-prow Bot commented Oct 10, 2023

Uh oh!

Cali0707 commented Oct 10, 2023

Uh oh!

codecov Bot commented Oct 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Cali0707 commented Oct 11, 2023

Uh oh!

pierDipi Oct 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pierDipi Oct 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pierDipi Oct 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Cali0707 commented Oct 13, 2023

Uh oh!

Cali0707 commented Oct 18, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

AnyFilter

AllFilter

Uh oh!

Cali0707 commented Nov 15, 2023

Uh oh!

knative-prow Bot commented Nov 15, 2023

Uh oh!

Cali0707 commented Dec 19, 2023

Uh oh!

github-actions Bot commented Mar 19, 2024

Uh oh!

github-actions Bot commented Jun 18, 2024

Uh oh!

Cali0707 commented Jun 20, 2024

Uh oh!

knative-prow-robot commented Jul 27, 2024

Uh oh!

github-actions Bot commented Dec 2, 2024

Uh oh!

github-actions Bot commented Mar 4, 2025

Uh oh!

github-actions Bot commented Jun 3, 2025

Uh oh!

github-actions Bot commented Sep 3, 2025

Uh oh!

github-actions Bot commented Dec 3, 2025

Uh oh!

github-actions Bot commented Mar 4, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov Bot commented Oct 10, 2023 •

edited

Loading

pierDipi Oct 11, 2023 •

edited

Loading

pierDipi Oct 11, 2023 •

edited

Loading

pierDipi Oct 11, 2023 •

edited

Loading

Cali0707 commented Oct 18, 2023 •

edited

Loading