You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by "orpiske (via GitHub)" <gi...@apache.org> on 2023/02/27 11:53:23 UTC
[GitHub] [camel] orpiske opened a new pull request, #9426: (perf chores) camel-seda: cache queue reference to avoid costly operations in the hot path
orpiske opened a new pull request, #9426:
URL: https://github.com/apache/camel/pull/9426
This one seems to add a very small (but still measurable) throughput improvement for Seda by avoiding a few synchronized calls. It's part of several other perf patches I will be sending
Compared w/ 4.0.0-M1:
--- Path
Mean: 484459.8571428573
Geometric mean: 484272.99632561835
Standard deviation: 13570.203212282813
--- 4.0.0-M1
Mean: 480957.58035714296
Geometric mean: 479545.8325907664
Standard deviation: 33212.22104132737
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] github-actions[bot] commented on pull request #9426: (perf chores) camel-seda: cache queue reference to avoid costly operations in the hot path
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9426:
URL: https://github.com/apache/camel/pull/9426#issuecomment-1446755007
### Components tested:
| Total | Tested | Failed :x: | Passed :white_check_mark: |
| --- | --- | --- | --- |
| 5 | 5 | 1 | 5 |
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] github-actions[bot] commented on pull request #9426: (perf chores) camel-seda: cache queue reference to avoid costly operations in the hot path
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9426:
URL: https://github.com/apache/camel/pull/9426#issuecomment-1446274627
### Components tested:
| Total | Tested | Failed :x: | Passed :white_check_mark: |
| --- | --- | --- | --- |
| 1 | 1 | 0 | 1 |
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] orpiske merged pull request #9426: (perf chores) camel-seda: cache queue reference to avoid costly operations in the hot path
Posted by "orpiske (via GitHub)" <gi...@apache.org>.
orpiske merged PR #9426:
URL: https://github.com/apache/camel/pull/9426
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] davsclaus commented on a diff in pull request #9426: (perf chores) camel-seda: cache queue reference to avoid costly operations in the hot path
Posted by "davsclaus (via GitHub)" <gi...@apache.org>.
davsclaus commented on code in PR #9426:
URL: https://github.com/apache/camel/pull/9426#discussion_r1118669914
##########
components/camel-seda/src/main/java/org/apache/camel/component/seda/SedaEndpoint.java:
##########
@@ -213,8 +215,14 @@ protected BlockingQueue<Exchange> createQueue() {
* @return the reference, or <tt>null</tt> if no queue reference exists.
*/
public synchronized QueueReference getQueueReference() {
- String key = getComponent().getQueueKey(getEndpointUri());
- QueueReference ref = getComponent().getQueueReference(key);
+ if (key == null) {
Review Comment:
I wonder if this can be done in doStart - I assume getEndpointUri is static at the moment when its started, and maybe then this can be computed once, and then the synchronization may not be need at all
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] orpiske commented on a diff in pull request #9426: (perf chores) camel-seda: cache queue reference to avoid costly operations in the hot path
Posted by "orpiske (via GitHub)" <gi...@apache.org>.
orpiske commented on code in PR #9426:
URL: https://github.com/apache/camel/pull/9426#discussion_r1118960436
##########
components/camel-seda/src/main/java/org/apache/camel/component/seda/SedaEndpoint.java:
##########
@@ -100,6 +100,8 @@ public class SedaEndpoint extends DefaultEndpoint implements AsyncEndpoint, Brow
private boolean discardIfNoConsumers;
private BlockingQueueFactory<Exchange> queueFactory;
+ private String key;
+ private QueueReference ref;
Review Comment:
I've just checked the code. We do have a few occasions where this could happen. Let's make it `volatile` here. Thanks @essobedo!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] github-actions[bot] commented on pull request #9426: (perf chores) camel-seda: cache queue reference to avoid costly operations in the hot path
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9426:
URL: https://github.com/apache/camel/pull/9426#issuecomment-1446718593
### Components tested:
| Total | Tested | Failed :x: | Passed :white_check_mark: |
| --- | --- | --- | --- |
| 1 | 1 | 0 | 1 |
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] essobedo commented on a diff in pull request #9426: (perf chores) camel-seda: cache queue reference to avoid costly operations in the hot path
Posted by "essobedo (via GitHub)" <gi...@apache.org>.
essobedo commented on code in PR #9426:
URL: https://github.com/apache/camel/pull/9426#discussion_r1118842612
##########
components/camel-seda/src/main/java/org/apache/camel/component/seda/SedaEndpoint.java:
##########
@@ -100,6 +100,8 @@ public class SedaEndpoint extends DefaultEndpoint implements AsyncEndpoint, Brow
private boolean discardIfNoConsumers;
private BlockingQueueFactory<Exchange> queueFactory;
+ private String key;
+ private QueueReference ref;
Review Comment:
Only `ref` seems to be needed here (`key` can be a local variable). Moreover, please note that it should be `volatile` otherwise you have unfortunately no guarantee that its value will be visible from a thread executed by a different core
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] github-actions[bot] commented on pull request #9426: (perf chores) camel-seda: cache queue reference to avoid costly operations in the hot path
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9426:
URL: https://github.com/apache/camel/pull/9426#issuecomment-1446194228
:star2: Thank you for your contribution to the Apache Camel project! :star2:
:warning: Please note that the changes on this PR may be **tested automatically**.
If necessary Apache Camel Committers may access logs and test results in the job summaries!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] orpiske commented on a diff in pull request #9426: (perf chores) camel-seda: cache queue reference to avoid costly operations in the hot path
Posted by "orpiske (via GitHub)" <gi...@apache.org>.
orpiske commented on code in PR #9426:
URL: https://github.com/apache/camel/pull/9426#discussion_r1118672499
##########
components/camel-seda/src/main/java/org/apache/camel/component/seda/SedaEndpoint.java:
##########
@@ -213,8 +215,14 @@ protected BlockingQueue<Exchange> createQueue() {
* @return the reference, or <tt>null</tt> if no queue reference exists.
*/
public synchronized QueueReference getQueueReference() {
- String key = getComponent().getQueueKey(getEndpointUri());
- QueueReference ref = getComponent().getQueueReference(key);
+ if (key == null) {
Review Comment:
🤔 Thanks! Sounds like a good idea and I can give it a try.
I'll dispatch the other PRs I have on the queue and then I'll go back to this one.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] orpiske commented on a diff in pull request #9426: (perf chores) camel-seda: cache queue reference to avoid costly operations in the hot path
Posted by "orpiske (via GitHub)" <gi...@apache.org>.
orpiske commented on code in PR #9426:
URL: https://github.com/apache/camel/pull/9426#discussion_r1118885643
##########
components/camel-seda/src/main/java/org/apache/camel/component/seda/SedaEndpoint.java:
##########
@@ -100,6 +100,8 @@ public class SedaEndpoint extends DefaultEndpoint implements AsyncEndpoint, Brow
private boolean discardIfNoConsumers;
private BlockingQueueFactory<Exchange> queueFactory;
+ private String key;
+ private QueueReference ref;
Review Comment:
+1 for `key`.
As for `ref`. Is there any circumstance where it could change after initialization (`start()`) @davsclaus? I think not ... so, we could probably get away with not using `volatile` here.
I would like very much to avoid the extra (very small, I know) cost of volatile here if possible.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] essobedo commented on a diff in pull request #9426: (perf chores) camel-seda: cache queue reference to avoid costly operations in the hot path
Posted by "essobedo (via GitHub)" <gi...@apache.org>.
essobedo commented on code in PR #9426:
URL: https://github.com/apache/camel/pull/9426#discussion_r1118842612
##########
components/camel-seda/src/main/java/org/apache/camel/component/seda/SedaEndpoint.java:
##########
@@ -100,6 +100,8 @@ public class SedaEndpoint extends DefaultEndpoint implements AsyncEndpoint, Brow
private boolean discardIfNoConsumers;
private BlockingQueueFactory<Exchange> queueFactory;
+ private String key;
+ private QueueReference ref;
Review Comment:
Only `ref` seems to be needed here. Moreover, please note that it should be `volatile` otherwise you have unfortunately no guarantee that its value will be visible from a thread executed by a different core
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] github-actions[bot] commented on pull request #9426: (perf chores) camel-seda: cache queue reference to avoid costly operations in the hot path
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9426:
URL: https://github.com/apache/camel/pull/9426#issuecomment-1446491583
### Components tested:
| Total | Tested | Failed :x: | Passed :white_check_mark: |
| --- | --- | --- | --- |
| 4 | 4 | 1 | 4 |
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] orpiske commented on a diff in pull request #9426: (perf chores) camel-seda: cache queue reference to avoid costly operations in the hot path
Posted by "orpiske (via GitHub)" <gi...@apache.org>.
orpiske commented on code in PR #9426:
URL: https://github.com/apache/camel/pull/9426#discussion_r1118780252
##########
components/camel-seda/src/main/java/org/apache/camel/component/seda/SedaEndpoint.java:
##########
@@ -213,8 +215,14 @@ protected BlockingQueue<Exchange> createQueue() {
* @return the reference, or <tt>null</tt> if no queue reference exists.
*/
public synchronized QueueReference getQueueReference() {
- String key = getComponent().getQueueKey(getEndpointUri());
- QueueReference ref = getComponent().getQueueReference(key);
+ if (key == null) {
Review Comment:
It turns out we can do that. I updated the PR with your suggestion, thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org