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/04/29 08:45:11 UTC
[GitHub] [camel] orpiske opened a new pull request, #9968: CAMEL-19058: rework the Message to avoid hitting the type-check scalability issue
orpiske opened a new pull request, #9968:
URL: https://github.com/apache/camel/pull/9968
This introduces a trait that works similarly to the internal properties one in the Exchange so that we carry certain details about the message without relying on type specifications.
Among other things this:
- avoids unnecessarily recreating the DefaultMessage multiple times
- moved the data type aware checks out of the hot path
- reworks the isTransactedRedelivered to avoid costlier checks for specific message types
This superseds the proposed changes on #9432.
Additionally, this implements the same pattern of using an array for
internal properties as we already do on the Exchange. Subsequently, my
idea is to abstract this complex array manipulation in an specific class
so we can reuse code and simplify maintenance.
--
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 #9968: CAMEL-19058: rework the Message to avoid hitting the type-check scalability issue
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9968:
URL: https://github.com/apache/camel/pull/9968#issuecomment-1529584028
### Components tested:
| Total | Tested | Failed :x: | Passed :white_check_mark: |
| --- | --- | --- | --- |
| 3 | 3 | 1 | 3 |
--
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 #9968: CAMEL-19058: rework the Message to avoid hitting the type-check scalability issue
Posted by "orpiske (via GitHub)" <gi...@apache.org>.
orpiske commented on code in PR #9968:
URL: https://github.com/apache/camel/pull/9968#discussion_r1181021055
##########
core/camel-support/src/main/java/org/apache/camel/support/MessageSupport.java:
##########
@@ -306,4 +310,20 @@ protected String createMessageId() {
}
}
+ @Override
+ public boolean hasTrait(MessageTrait trait) {
+ Object payload = traits[trait.ordinal()];
+
+ return payload != null;
+ }
+
+ @Override
+ public Object getPayloadForTrait(MessageTrait trait) {
+ return traits[trait.ordinal()];
+ }
+
+ @Override
+ public void setPayloadForTrait(MessageTrait trait, Object object) {
+ traits[trait.ordinal()] = object;
+ }
Review Comment:
This part here: if accepted, then I'll do create a separate class to abstract this. We already do a similar thing when handling the `internalProperties` on the [AbstractExchange](https://github.com/apache/camel/blob/main/core/camel-support/src/main/java/org/apache/camel/support/AbstractExchange.java#L252-L276).
--
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 #9968: CAMEL-19058: rework the Message to avoid hitting the type-check scalability issue
Posted by "orpiske (via GitHub)" <gi...@apache.org>.
orpiske commented on PR #9968:
URL: https://github.com/apache/camel/pull/9968#issuecomment-1529062170
> I wonder if we should move some of those trait out of the root package to avoid having to much stuff there (there is already too much) - as those are mostly for internal optimization use, then they better fit in .spi or in a new .trait package.
I like this idea. Let me explore it in one shot with this PR.
Also, I'm marking this one as a draft for a while. I found a bug in the JMS code and I'll need to review it.
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
[GitHub] [camel] github-actions[bot] commented on pull request #9968: CAMEL-19058: rework the Message to avoid hitting the type-check scalability issue
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9968:
URL: https://github.com/apache/camel/pull/9968#issuecomment-1529750604
### Components tested:
| Total | Tested | Failed :x: | Passed :white_check_mark: |
| --- | --- | --- | --- |
| 3 | 3 | 0 | 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] github-actions[bot] commented on pull request #9968: CAMEL-19058: rework the Message to avoid hitting the type-check scalability issue
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9968:
URL: https://github.com/apache/camel/pull/9968#issuecomment-1529743471
### Components tested:
| Total | Tested | Failed :x: | Passed :white_check_mark: |
| --- | --- | --- | --- |
| 3 | 3 | 1 | 3 |
--
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 #9968: CAMEL-19058: rework the Message to avoid hitting the type-check scalability issue
Posted by "orpiske (via GitHub)" <gi...@apache.org>.
orpiske commented on code in PR #9968:
URL: https://github.com/apache/camel/pull/9968#discussion_r1181021055
##########
core/camel-support/src/main/java/org/apache/camel/support/MessageSupport.java:
##########
@@ -306,4 +310,20 @@ protected String createMessageId() {
}
}
+ @Override
+ public boolean hasTrait(MessageTrait trait) {
+ Object payload = traits[trait.ordinal()];
+
+ return payload != null;
+ }
+
+ @Override
+ public Object getPayloadForTrait(MessageTrait trait) {
+ return traits[trait.ordinal()];
+ }
+
+ @Override
+ public void setPayloadForTrait(MessageTrait trait, Object object) {
+ traits[trait.ordinal()] = object;
+ }
Review Comment:
This part here: if accepted, then I'll do create a separate class to abstract this. We already do a similar thing when handling the `internalProperties` on the [AbstractExchange](https://github.com/apache/camel/blob/main/core/camel-support/src/main/java/org/apache/camel/support/AbstractExchange.java#L252-L276). So, there's room for future cleanup and consolidation of the code base.
--
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 pull request #9968: CAMEL-19058: rework the Message to avoid hitting the type-check scalability issue
Posted by "essobedo (via GitHub)" <gi...@apache.org>.
essobedo commented on PR #9968:
URL: https://github.com/apache/camel/pull/9968#issuecomment-1530972335
By chance do you have any benchmarks to share?
--
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 #9968: CAMEL-19058: rework the Message to avoid hitting the type-check scalability issue
Posted by "orpiske (via GitHub)" <gi...@apache.org>.
orpiske merged PR #9968:
URL: https://github.com/apache/camel/pull/9968
--
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 #9968: CAMEL-19058: rework the Message to avoid hitting the type-check scalability issue
Posted by "orpiske (via GitHub)" <gi...@apache.org>.
orpiske commented on PR #9968:
URL: https://github.com/apache/camel/pull/9968#issuecomment-1530980188
> By chance do you have any benchmarks to share?
Yes I do. I am preparing a blog post with the details, methodology, etc. But here's a preview for the disruptor (in just few of the many other scenarios):
Obs.: the notation is inverted and the baseline is the code is the code with this patch and the test code is the test with 4.0.0-M1.
*25 consumers + 25 producers*:
```
GEOMETRIC MEAN (RATE):
Test: 640260.6763745428 | Baseline: 712828.9356009845
Delta: -72568.25922644173
The geometric mean rate was lower for the test than for the baseline
```
*10 consumers + 9 producers*:
```
GEOMETRIC MEAN (RATE):
Test: 767544.7450252476 | Baseline: 841304.9248985515
Delta: -73760.17987330398
The geometric mean rate was lower for the test than for the baseline
```
*7 consumers + 3 producers*:
```
GEOMETRIC MEAN (RATE):
Test: 486716.1190278101 | Baseline: 610061.3909864418
Delta: -123345.27195863164
The geometric mean rate was lower for the test than for the baseline
```
All in all, the new code tends to be faster in around 95% of the cases. The results from JMH also indicate the code being faster (i.e.; having a higher score in ops/ms - IIRC), although I don't have them easily at hand right now.
--
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 #9968: CAMEL-19058: rework the Message to avoid hitting the type-check scalability issue
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9968:
URL: https://github.com/apache/camel/pull/9968#issuecomment-1528720863
: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] github-actions[bot] commented on pull request #9968: CAMEL-19058: rework the Message to avoid hitting the type-check scalability issue
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9968:
URL: https://github.com/apache/camel/pull/9968#issuecomment-1528732812
### Components tested:
| Total | Tested | Failed :x: | Passed :white_check_mark: |
| --- | --- | --- | --- |
| 3 | 3 | 3 | 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] davsclaus commented on pull request #9968: CAMEL-19058: rework the Message to avoid hitting the type-check scalability issue
Posted by "davsclaus (via GitHub)" <gi...@apache.org>.
davsclaus commented on PR #9968:
URL: https://github.com/apache/camel/pull/9968#issuecomment-1529031720
I wonder if we should move some of those trait out of the root package to avoid having to much stuff there (there is already too much) - as those are mostly for internal optimization use, then they better fit in .spi or in a new .trait package.
--
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 #9968: CAMEL-19058: rework the Message to avoid hitting the type-check scalability issue
Posted by "orpiske (via GitHub)" <gi...@apache.org>.
orpiske commented on PR #9968:
URL: https://github.com/apache/camel/pull/9968#issuecomment-1529523817
Remarking as ready to review after fixing a bug in JMS changes. Also moved the traits to a separate `trait` package.
--
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 #9968: CAMEL-19058: rework the Message to avoid hitting the type-check scalability issue
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9968:
URL: https://github.com/apache/camel/pull/9968#issuecomment-1529563239
### Components tested:
| Total | Tested | Failed :x: | Passed :white_check_mark: |
| --- | --- | --- | --- |
| 3 | 3 | 4 | 0 |
--
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