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 12:23:57 UTC

[GitHub] [camel] orpiske opened a new pull request, #9429: CAMEL-19060: move the isFailureHandled logic to a field in the ExchangeExtension to avoid costly operations

orpiske opened a new pull request, #9429:
URL: https://github.com/apache/camel/pull/9429

   This is a small optimization aimed at that avoiding costly operations in AbstractExchange#getProperty. It brings a small but noticeable improvement to SEDA (and probably others): 
   
   ```
   -- Patch
   Mean: 457300.04464285704
   Geometric mean: 457246.17949513963
   Standard deviation: 7040.578210525716
   
   -- 4.0.0-M1
   Maximum: 490420.0
   Mean: 453472.6964285715
   Geometric mean: 453389.6965851105
   Standard deviation: 8802.284218119967
   ```


-- 
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 #9429: CAMEL-19060: move the isFailureHandled logic to a field in the ExchangeExtension to avoid costly operations

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9429:
URL: https://github.com/apache/camel/pull/9429#issuecomment-1446238380

   :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 #9429: CAMEL-19060: move the isFailureHandled logic to a field in the ExchangeExtension to avoid costly operations

Posted by "orpiske (via GitHub)" <gi...@apache.org>.
orpiske commented on code in PR #9429:
URL: https://github.com/apache/camel/pull/9429#discussion_r1118677015


##########
core/camel-support/src/main/java/org/apache/camel/support/ExtendedExchangeExtension.java:
##########
@@ -30,6 +30,7 @@
 
 public class ExtendedExchangeExtension implements ExchangeExtension {
     private final AbstractExchange exchange;
+    private boolean failureHandled;

Review Comment:
   Noted, thanks! I'll take a look at that. 



-- 
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 #9429: CAMEL-19060: move the isFailureHandled logic to a field in the ExchangeExtension to avoid costly operations

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9429:
URL: https://github.com/apache/camel/pull/9429#issuecomment-1446321005

   :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 #9429: CAMEL-19060: move the isFailureHandled logic to a field in the ExchangeExtension to avoid costly operations

Posted by "davsclaus (via GitHub)" <gi...@apache.org>.
davsclaus commented on code in PR #9429:
URL: https://github.com/apache/camel/pull/9429#discussion_r1118673996


##########
core/camel-support/src/main/java/org/apache/camel/support/ExtendedExchangeExtension.java:
##########
@@ -30,6 +30,7 @@
 
 public class ExtendedExchangeExtension implements ExchangeExtension {
     private final AbstractExchange exchange;
+    private boolean failureHandled;

Review Comment:
   The other flags are in AbstractExchange, so maybe we should move it to where they are.



-- 
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 #9429: CAMEL-19060: move the isFailureHandled logic to a field in the ExchangeExtension to avoid costly operations

Posted by "orpiske (via GitHub)" <gi...@apache.org>.
orpiske commented on PR #9429:
URL: https://github.com/apache/camel/pull/9429#issuecomment-1446238928

   @davsclaus @oscerd I guess one of you might one to double check 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 #9429: CAMEL-19060: move the isFailureHandled logic to a field in the ExchangeExtension to avoid costly operations

Posted by "davsclaus (via GitHub)" <gi...@apache.org>.
davsclaus commented on code in PR #9429:
URL: https://github.com/apache/camel/pull/9429#discussion_r1118678793


##########
core/camel-core-processor/src/main/java/org/apache/camel/processor/OnCompletionProcessor.java:
##########
@@ -190,8 +190,8 @@ protected static void doProcess(Processor processor, Exchange exchange) {
         } finally {
             // restore the options
             exchange.setRouteStop(stop);
-            if (failureHandled != null) {
-                exchange.setProperty(ExchangePropertyKey.FAILURE_HANDLED, failureHandled);
+            if (failureHandled) {

Review Comment:
   I wonder if we have anywhere in the code, where we call: setFailureHandled(false), as then this code will not restore that value.
   
   However I suspect we do not have that, as we use Boolean.TRUE before, and "remove" or "null" to say false/none. But may be worth to grep the source to find what we do



-- 
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 #9429: CAMEL-19060: move the isFailureHandled logic to a field in the ExchangeExtension to avoid costly operations

Posted by "orpiske (via GitHub)" <gi...@apache.org>.
orpiske commented on code in PR #9429:
URL: https://github.com/apache/camel/pull/9429#discussion_r1118684129


##########
core/camel-core-processor/src/main/java/org/apache/camel/processor/OnCompletionProcessor.java:
##########
@@ -190,8 +190,8 @@ protected static void doProcess(Processor processor, Exchange exchange) {
         } finally {
             // restore the options
             exchange.setRouteStop(stop);
-            if (failureHandled != null) {
-                exchange.setProperty(ExchangePropertyKey.FAILURE_HANDLED, failureHandled);
+            if (failureHandled) {

Review Comment:
   It seems we don't do that. Neither IntelliJ nor `egrep -inR "setFailureHandled" *` found any instance of us setting the flag to false. I believe we should be fine with 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 merged pull request #9429: CAMEL-19060: move the isFailureHandled logic to a field in the ExchangeExtension to avoid costly operations

Posted by "orpiske (via GitHub)" <gi...@apache.org>.
orpiske merged PR #9429:
URL: https://github.com/apache/camel/pull/9429


-- 
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 #9429: CAMEL-19060: move the isFailureHandled logic to a field in the ExchangeExtension to avoid costly operations

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9429:
URL: https://github.com/apache/camel/pull/9429#issuecomment-1446738820

   :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