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/05/03 12:00:52 UTC
[GitHub] [camel] orpiske opened a new pull request, #9986: CAMEL-19058: cache the producer and endpoint locally to avoid hitting the type check scalability issue"
orpiske opened a new pull request, #9986:
URL: https://github.com/apache/camel/pull/9986
This reworks the patch originally added on 907a9d0c08d40c5308bb440ec9ebaaa4d7544dc4 so that there is no side effect when handling singleton endpoints
--
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 #9986: CAMEL-19058: cache the producer and endpoint locally to avoid hitting the type check scalability issue
Posted by "orpiske (via GitHub)" <gi...@apache.org>.
orpiske commented on code in PR #9986:
URL: https://github.com/apache/camel/pull/9986#discussion_r1183687861
##########
core/camel-support/src/main/java/org/apache/camel/support/cache/DefaultProducerCache.java:
##########
@@ -120,11 +123,20 @@ public Object getSource() {
@Override
public AsyncProducer acquireProducer(Endpoint endpoint) {
+ // Try to favor thread locality as some data in the producer's cache may be shared among threads,
+ // triggering cases of false sharing
+ if (endpoint == lastUsedEndpoint && endpoint.isSingletonProducer()) {
+ return lastUsedProducer;
+ }
+
try {
AsyncProducer producer = producers.acquire(endpoint);
if (statistics != null) {
statistics.onHit(endpoint.getEndpointUri());
}
+ lastUsedEndpoint = endpoint;
Review Comment:
Hm, I guess I should synchronize this, then ...
--
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 #9986: CAMEL-19058: cache the producer and endpoint locally to avoid hitting the type check scalability issue"
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9986:
URL: https://github.com/apache/camel/pull/9986#issuecomment-1532901652
:star2: Thank you for your contribution to the Apache Camel project! :star2:
:camel: Maintainers, please note that first-time contributors *require manual approval* for the GitHub Actions to run.
:warning: Please note that the changes on this PR may be **tested automatically** if they change components.
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 merged pull request #9986: CAMEL-19102: cache the producer and endpoint locally to avoid hitting the type check scalability issue
Posted by "orpiske (via GitHub)" <gi...@apache.org>.
orpiske merged PR #9986:
URL: https://github.com/apache/camel/pull/9986
--
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 pull request #9986: CAMEL-19058: cache the producer and endpoint locally to avoid hitting the type check scalability issue
Posted by "orpiske (via GitHub)" <gi...@apache.org>.
orpiske commented on PR #9986:
URL: https://github.com/apache/camel/pull/9986#issuecomment-1547509177
I am back at looking at this one. The patch is (likely) fine, but I want to review the results one more time.
--
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 #9986: CAMEL-19058: cache the producer and endpoint locally to avoid hitting the type check scalability issue
Posted by "orpiske (via GitHub)" <gi...@apache.org>.
orpiske commented on code in PR #9986:
URL: https://github.com/apache/camel/pull/9986#discussion_r1183728748
##########
core/camel-support/src/main/java/org/apache/camel/support/cache/DefaultProducerCache.java:
##########
@@ -120,11 +123,20 @@ public Object getSource() {
@Override
public AsyncProducer acquireProducer(Endpoint endpoint) {
+ // Try to favor thread locality as some data in the producer's cache may be shared among threads,
+ // triggering cases of false sharing
+ if (endpoint == lastUsedEndpoint && endpoint.isSingletonProducer()) {
+ return lastUsedProducer;
+ }
+
try {
AsyncProducer producer = producers.acquire(endpoint);
if (statistics != null) {
statistics.onHit(endpoint.getEndpointUri());
}
+ lastUsedEndpoint = endpoint;
Review Comment:
Thanks for pointing this out. I've made change ...
--
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 pull request #9986: CAMEL-19058: cache the producer and endpoint locally to avoid hitting the type check scalability issue
Posted by "orpiske (via GitHub)" <gi...@apache.org>.
orpiske commented on PR #9986:
URL: https://github.com/apache/camel/pull/9986#issuecomment-1535720489
Folks, I am marking this one as a draft. I am thinking about linking a few related PRs, so it may be easier to look at the whole set of changes together.
--
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 #9986: CAMEL-19058: cache the producer and endpoint locally to avoid hitting the type check scalability issue
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9986:
URL: https://github.com/apache/camel/pull/9986#issuecomment-1532975165
:no_entry_sign: There are (likely) no components to be tested in this PR
--
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 #9986: CAMEL-19058: cache the producer and endpoint locally to avoid hitting the type check scalability issue
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9986:
URL: https://github.com/apache/camel/pull/9986#issuecomment-1532974855
:no_entry_sign: There are (likely) no components to be tested in this PR
--
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 #9986: CAMEL-19058: cache the producer and endpoint locally to avoid hitting the type check scalability issue
Posted by "davsclaus (via GitHub)" <gi...@apache.org>.
davsclaus commented on code in PR #9986:
URL: https://github.com/apache/camel/pull/9986#discussion_r1183685358
##########
core/camel-support/src/main/java/org/apache/camel/support/cache/DefaultProducerCache.java:
##########
@@ -120,11 +123,20 @@ public Object getSource() {
@Override
public AsyncProducer acquireProducer(Endpoint endpoint) {
+ // Try to favor thread locality as some data in the producer's cache may be shared among threads,
+ // triggering cases of false sharing
+ if (endpoint == lastUsedEndpoint && endpoint.isSingletonProducer()) {
+ return lastUsedProducer;
+ }
+
try {
AsyncProducer producer = producers.acquire(endpoint);
if (statistics != null) {
statistics.onHit(endpoint.getEndpointUri());
}
+ lastUsedEndpoint = endpoint;
Review Comment:
You could potentially have a race condition where thread A changes these 2 fields, and thread B does that as well, and then last endpoint and last producer could get mixed up.
--
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 #9986: CAMEL-19102: cache the producer and endpoint locally to avoid hitting the type check scalability issue
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9986:
URL: https://github.com/apache/camel/pull/9986#issuecomment-1547605399
:no_entry_sign: There are (likely) no components to be tested in this PR
--
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 #9986: CAMEL-19058: cache the producer and endpoint locally to avoid hitting the type check scalability issue
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9986:
URL: https://github.com/apache/camel/pull/9986#issuecomment-1532978636
:no_entry_sign: There are (likely) no components to be tested in this PR
--
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 #9986: CAMEL-19058: cache the producer and endpoint locally to avoid hitting the type check scalability issue
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9986:
URL: https://github.com/apache/camel/pull/9986#issuecomment-1533214231
### Components tested:
| Total | Tested | Failed :x: | Passed :white_check_mark: |
| --- | --- | --- | --- |
| 1 | 1 | 0 | 2 |
--
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 pull request #9986: CAMEL-19102: cache the producer and endpoint locally to avoid hitting the type check scalability issue
Posted by "orpiske (via GitHub)" <gi...@apache.org>.
orpiske commented on PR #9986:
URL: https://github.com/apache/camel/pull/9986#issuecomment-1547821549
Just waiting for the JMH to run and we should be good to go (above vs. M1)
--
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