Uploaded image for project: 'CXF'
  1. CXF
  2. CXF-7716

IBM Performance Team has found several performance increases

Details

    • Bug
    • Status: Closed
    • Major
    • Resolution: Fixed
    • 3.1.15, 3.2.4
    • 3.1.16, 3.2.5
    • JAX-RS
    • None
    • Moderate

    Description

      Our performance team has identified several areas of improvement to reduce garbage collection and CPU usage.

      First, we reduced the amount of StringBuilders created in HTTPUtils.java and ResourceUtils.java.

      Second, we created JAXRSUtils.doMimeTypesIntersect() - a method similar to JAXRSUtils.intersectMimeTypes - that doesn't create a HashSet but instead returns a boolean when we just need to know if they intersect.

      Third, we found that getting the annotations for parameters to create constructor arguments in PerRequestResourceProvider.java is expensive, so we cache them in the constructor instead of getting them via reflection every request.

      These changes combined result in a ~1.5-2% performance increase.

      Attachments

        Issue Links

          Activity

            githubbot ASF GitHub Bot added a comment -

            WhiteCat22 opened a new pull request #407: CXF-7716 Reduce StringBuilders and other performance changes.
            URL: https://github.com/apache/cxf/pull/407

            The IBM performance team has identified several areas of improvement to reduce garbage collection and CPU usage.

            First, we reduced the amount of StringBuilders created in HTTPUtils.java and ResourceUtils.java.

            Second, we created JAXRSUtils.doMimeTypesIntersect() - a method similar to JAXRSUtils.intersectMimeTypes - that doesn't create a HashSet but instead returns a boolean when we just need to know if they intersect.

            Third, we found that getting the annotations for parameters to create constructor arguments in PerRequestResourceProvider.java is expensive, so we cache them in the constructor instead of getting them via reflection every request.

            These changes combined result in a ~1.5-2% performance increase.

            ----------------------------------------------------------------
            This is an automated message from the Apache Git Service.
            To respond to the message, please log on GitHub and use the
            URL above to go to the specific comment.

            For queries about this service, please contact Infrastructure at:
            users@infra.apache.org

            githubbot ASF GitHub Bot added a comment - WhiteCat22 opened a new pull request #407: CXF-7716 Reduce StringBuilders and other performance changes. URL: https://github.com/apache/cxf/pull/407 The IBM performance team has identified several areas of improvement to reduce garbage collection and CPU usage. First, we reduced the amount of StringBuilders created in HTTPUtils.java and ResourceUtils.java. Second, we created JAXRSUtils.doMimeTypesIntersect() - a method similar to JAXRSUtils.intersectMimeTypes - that doesn't create a HashSet but instead returns a boolean when we just need to know if they intersect. Third, we found that getting the annotations for parameters to create constructor arguments in PerRequestResourceProvider.java is expensive, so we cache them in the constructor instead of getting them via reflection every request. These changes combined result in a ~1.5-2% performance increase. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: users@infra.apache.org
            githubbot ASF GitHub Bot added a comment -

            reta commented on a change in pull request #407: CXF-7716 Reduce StringBuilders and other performance changes.
            URL: https://github.com/apache/cxf/pull/407#discussion_r182614831

            ##########
            File path: rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/ResourceUtils.java
            ##########
            @@ -816,12 +816,23 @@ private static UserOperation getOperationFromElement(Element e) {
            Message m,
            boolean perRequest,
            Map<Class<?>, Object> contextValues) {

            • if (m == null) { - m = new MessageImpl(); - }

              Class<?>[] params = c.getParameterTypes();
              Annotation[][] anns = c.getParameterAnnotations();
              Type[] genericTypes = c.getGenericParameterTypes();
              + return createConstructorArguments(c, m, perRequest, contextValues, params, anns, genericTypes);
              + }
              +
              + public static Object[] createConstructorArguments(Constructor<?> c,

            Review comment:
            Are those new arguments (`params`, `anns`, `genericTypes`) used by the method?

            ----------------------------------------------------------------
            This is an automated message from the Apache Git Service.
            To respond to the message, please log on GitHub and use the
            URL above to go to the specific comment.

            For queries about this service, please contact Infrastructure at:
            users@infra.apache.org

            githubbot ASF GitHub Bot added a comment - reta commented on a change in pull request #407: CXF-7716 Reduce StringBuilders and other performance changes. URL: https://github.com/apache/cxf/pull/407#discussion_r182614831 ########## File path: rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/ResourceUtils.java ########## @@ -816,12 +816,23 @@ private static UserOperation getOperationFromElement(Element e) { Message m, boolean perRequest, Map<Class<?>, Object> contextValues) { if (m == null) { - m = new MessageImpl(); - } Class<?>[] params = c.getParameterTypes(); Annotation[][] anns = c.getParameterAnnotations(); Type[] genericTypes = c.getGenericParameterTypes(); + return createConstructorArguments(c, m, perRequest, contextValues, params, anns, genericTypes); + } + + public static Object[] createConstructorArguments(Constructor<?> c, Review comment: Are those new arguments (`params`, `anns`, `genericTypes`) used by the method? ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: users@infra.apache.org
            githubbot ASF GitHub Bot added a comment -

            reta commented on a change in pull request #407: CXF-7716 Reduce StringBuilders and other performance changes.
            URL: https://github.com/apache/cxf/pull/407#discussion_r182615636

            ##########
            File path: rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/model/URITemplate.java
            ##########
            @@ -130,10 +130,31 @@ public String getPatternValue() {

            private static String escapeCharacters(String expression) {

            • StringBuilder sb = new StringBuilder();
            • for (int i = 0; i < expression.length(); i++) {
            • char ch = expression.charAt;
            • sb.append(isReservedCharacter(ch) ? "
              " + ch : ch);
              + int length = expression.length();

            Review comment:
            My apologies but this implementation is not very convincing to me, I think we could just pre-allocate `2 * expression.length()` using `StringBuilder ` constructor (worst case when all characters are reserved) and keep the previous one?

            ----------------------------------------------------------------
            This is an automated message from the Apache Git Service.
            To respond to the message, please log on GitHub and use the
            URL above to go to the specific comment.

            For queries about this service, please contact Infrastructure at:
            users@infra.apache.org

            githubbot ASF GitHub Bot added a comment - reta commented on a change in pull request #407: CXF-7716 Reduce StringBuilders and other performance changes. URL: https://github.com/apache/cxf/pull/407#discussion_r182615636 ########## File path: rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/model/URITemplate.java ########## @@ -130,10 +130,31 @@ public String getPatternValue() { private static String escapeCharacters(String expression) { StringBuilder sb = new StringBuilder(); for (int i = 0; i < expression.length(); i++) { char ch = expression.charAt ; sb.append(isReservedCharacter(ch) ? " " + ch : ch); + int length = expression.length(); Review comment: My apologies but this implementation is not very convincing to me, I think we could just pre-allocate `2 * expression.length()` using `StringBuilder ` constructor (worst case when all characters are reserved) and keep the previous one? ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: users@infra.apache.org
            githubbot ASF GitHub Bot added a comment -

            WhiteCat22 commented on a change in pull request #407: CXF-7716 Reduce StringBuilders and other performance changes.
            URL: https://github.com/apache/cxf/pull/407#discussion_r182756615

            ##########
            File path: rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/ResourceUtils.java
            ##########
            @@ -816,12 +816,23 @@ private static UserOperation getOperationFromElement(Element e) {
            Message m,
            boolean perRequest,
            Map<Class<?>, Object> contextValues) {

            • if (m == null) { - m = new MessageImpl(); - }

              Class<?>[] params = c.getParameterTypes();
              Annotation[][] anns = c.getParameterAnnotations();
              Type[] genericTypes = c.getGenericParameterTypes();
              + return createConstructorArguments(c, m, perRequest, contextValues, params, anns, genericTypes);
              + }
              +
              + public static Object[] createConstructorArguments(Constructor<?> c,

            Review comment:
            Yes, the idea is that we get them once in the PerRequestResourceProvider constructor, vs getting them via reflection every request.

            ----------------------------------------------------------------
            This is an automated message from the Apache Git Service.
            To respond to the message, please log on GitHub and use the
            URL above to go to the specific comment.

            For queries about this service, please contact Infrastructure at:
            users@infra.apache.org

            githubbot ASF GitHub Bot added a comment - WhiteCat22 commented on a change in pull request #407: CXF-7716 Reduce StringBuilders and other performance changes. URL: https://github.com/apache/cxf/pull/407#discussion_r182756615 ########## File path: rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/ResourceUtils.java ########## @@ -816,12 +816,23 @@ private static UserOperation getOperationFromElement(Element e) { Message m, boolean perRequest, Map<Class<?>, Object> contextValues) { if (m == null) { - m = new MessageImpl(); - } Class<?>[] params = c.getParameterTypes(); Annotation[][] anns = c.getParameterAnnotations(); Type[] genericTypes = c.getGenericParameterTypes(); + return createConstructorArguments(c, m, perRequest, contextValues, params, anns, genericTypes); + } + + public static Object[] createConstructorArguments(Constructor<?> c, Review comment: Yes, the idea is that we get them once in the PerRequestResourceProvider constructor, vs getting them via reflection every request. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: users@infra.apache.org
            githubbot ASF GitHub Bot added a comment -

            WhiteCat22 commented on a change in pull request #407: CXF-7716 Reduce StringBuilders and other performance changes.
            URL: https://github.com/apache/cxf/pull/407#discussion_r182758236

            ##########
            File path: rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/model/URITemplate.java
            ##########
            @@ -130,10 +130,31 @@ public String getPatternValue() {

            private static String escapeCharacters(String expression) {

            • StringBuilder sb = new StringBuilder();
            • for (int i = 0; i < expression.length(); i++) {
            • char ch = expression.charAt;
            • sb.append(isReservedCharacter(ch) ? "
              " + ch : ch);
              + int length = expression.length();

            Review comment:
            The goal is to reduce the number of StringBuilders created in order to reduce GCs and CPU usage. In this case we don't create a StringBuilder until we are sure that we actually need one, vs creating one that we might not even use.

            ----------------------------------------------------------------
            This is an automated message from the Apache Git Service.
            To respond to the message, please log on GitHub and use the
            URL above to go to the specific comment.

            For queries about this service, please contact Infrastructure at:
            users@infra.apache.org

            githubbot ASF GitHub Bot added a comment - WhiteCat22 commented on a change in pull request #407: CXF-7716 Reduce StringBuilders and other performance changes. URL: https://github.com/apache/cxf/pull/407#discussion_r182758236 ########## File path: rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/model/URITemplate.java ########## @@ -130,10 +130,31 @@ public String getPatternValue() { private static String escapeCharacters(String expression) { StringBuilder sb = new StringBuilder(); for (int i = 0; i < expression.length(); i++) { char ch = expression.charAt ; sb.append(isReservedCharacter(ch) ? " " + ch : ch); + int length = expression.length(); Review comment: The goal is to reduce the number of StringBuilders created in order to reduce GCs and CPU usage. In this case we don't create a StringBuilder until we are sure that we actually need one, vs creating one that we might not even use. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: users@infra.apache.org
            githubbot ASF GitHub Bot added a comment -

            reta commented on a change in pull request #407: CXF-7716 Reduce StringBuilders and other performance changes.
            URL: https://github.com/apache/cxf/pull/407#discussion_r182915164

            ##########
            File path: rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/ResourceUtils.java
            ##########
            @@ -816,12 +816,23 @@ private static UserOperation getOperationFromElement(Element e) {
            Message m,
            boolean perRequest,
            Map<Class<?>, Object> contextValues) {

            • if (m == null) { - m = new MessageImpl(); - }

              Class<?>[] params = c.getParameterTypes();
              Annotation[][] anns = c.getParameterAnnotations();
              Type[] genericTypes = c.getGenericParameterTypes();
              + return createConstructorArguments(c, m, perRequest, contextValues, params, anns, genericTypes);
              + }
              +
              + public static Object[] createConstructorArguments(Constructor<?> c,

            Review comment:
            Make sense, thank you.

            ----------------------------------------------------------------
            This is an automated message from the Apache Git Service.
            To respond to the message, please log on GitHub and use the
            URL above to go to the specific comment.

            For queries about this service, please contact Infrastructure at:
            users@infra.apache.org

            githubbot ASF GitHub Bot added a comment - reta commented on a change in pull request #407: CXF-7716 Reduce StringBuilders and other performance changes. URL: https://github.com/apache/cxf/pull/407#discussion_r182915164 ########## File path: rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/ResourceUtils.java ########## @@ -816,12 +816,23 @@ private static UserOperation getOperationFromElement(Element e) { Message m, boolean perRequest, Map<Class<?>, Object> contextValues) { if (m == null) { - m = new MessageImpl(); - } Class<?>[] params = c.getParameterTypes(); Annotation[][] anns = c.getParameterAnnotations(); Type[] genericTypes = c.getGenericParameterTypes(); + return createConstructorArguments(c, m, perRequest, contextValues, params, anns, genericTypes); + } + + public static Object[] createConstructorArguments(Constructor<?> c, Review comment: Make sense, thank you. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: users@infra.apache.org
            githubbot ASF GitHub Bot added a comment -

            reta commented on a change in pull request #407: CXF-7716 Reduce StringBuilders and other performance changes.
            URL: https://github.com/apache/cxf/pull/407#discussion_r182915735

            ##########
            File path: rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/model/URITemplate.java
            ##########
            @@ -130,10 +130,31 @@ public String getPatternValue() {

            private static String escapeCharacters(String expression) {

            • StringBuilder sb = new StringBuilder();
            • for (int i = 0; i < expression.length(); i++) {
            • char ch = expression.charAt;
            • sb.append(isReservedCharacter(ch) ? "
              " + ch : ch);
              + int length = expression.length();

            Review comment:
            Thanks, from this perspective we could consider it as micro-optimization, trading the code readability, I think it is acceptable in this case, the implementation could be understood easily.

            ----------------------------------------------------------------
            This is an automated message from the Apache Git Service.
            To respond to the message, please log on GitHub and use the
            URL above to go to the specific comment.

            For queries about this service, please contact Infrastructure at:
            users@infra.apache.org

            githubbot ASF GitHub Bot added a comment - reta commented on a change in pull request #407: CXF-7716 Reduce StringBuilders and other performance changes. URL: https://github.com/apache/cxf/pull/407#discussion_r182915735 ########## File path: rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/model/URITemplate.java ########## @@ -130,10 +130,31 @@ public String getPatternValue() { private static String escapeCharacters(String expression) { StringBuilder sb = new StringBuilder(); for (int i = 0; i < expression.length(); i++) { char ch = expression.charAt ; sb.append(isReservedCharacter(ch) ? " " + ch : ch); + int length = expression.length(); Review comment: Thanks, from this perspective we could consider it as micro-optimization, trading the code readability, I think it is acceptable in this case, the implementation could be understood easily. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: users@infra.apache.org
            githubbot ASF GitHub Bot added a comment -

            reta commented on a change in pull request #407: CXF-7716 Reduce StringBuilders and other performance changes.
            URL: https://github.com/apache/cxf/pull/407#discussion_r182916384

            ##########
            File path: rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/model/URITemplate.java
            ##########
            @@ -130,10 +130,31 @@ public String getPatternValue() {

            private static String escapeCharacters(String expression) {

            • StringBuilder sb = new StringBuilder();
            • for (int i = 0; i < expression.length(); i++) {
            • char ch = expression.charAt;
            • sb.append(isReservedCharacter(ch) ? "
              " + ch : ch);
              + int length = expression.length();
              + int i = 0;
              + char ch = ' ';
              + for (; i < length; ++i)
              Unknown macro: {+ ch = expression.charAt(i);+ if (isReservedCharacter(ch)) { + break; + }+ }

              +
              + if (i == length)

              { + return expression; + }

              +
              + StringBuilder sb = new StringBuilder(length + 8);

            Review comment:
            Why `length + 8`?

            ----------------------------------------------------------------
            This is an automated message from the Apache Git Service.
            To respond to the message, please log on GitHub and use the
            URL above to go to the specific comment.

            For queries about this service, please contact Infrastructure at:
            users@infra.apache.org

            githubbot ASF GitHub Bot added a comment - reta commented on a change in pull request #407: CXF-7716 Reduce StringBuilders and other performance changes. URL: https://github.com/apache/cxf/pull/407#discussion_r182916384 ########## File path: rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/model/URITemplate.java ########## @@ -130,10 +130,31 @@ public String getPatternValue() { private static String escapeCharacters(String expression) { StringBuilder sb = new StringBuilder(); for (int i = 0; i < expression.length(); i++) { char ch = expression.charAt ; sb.append(isReservedCharacter(ch) ? " " + ch : ch); + int length = expression.length(); + int i = 0; + char ch = ' '; + for (; i < length; ++i) Unknown macro: {+ ch = expression.charAt(i);+ if (isReservedCharacter(ch)) { + break; + }+ } + + if (i == length) { + return expression; + } + + StringBuilder sb = new StringBuilder(length + 8); Review comment: Why `length + 8`? ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: users@infra.apache.org
            githubbot ASF GitHub Bot added a comment -

            asoldano commented on a change in pull request #407: CXF-7716 Reduce StringBuilders and other performance changes.
            URL: https://github.com/apache/cxf/pull/407#discussion_r182964151

            ##########
            File path: rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/model/URITemplate.java
            ##########
            @@ -130,10 +130,31 @@ public String getPatternValue() {

            private static String escapeCharacters(String expression) {

            • StringBuilder sb = new StringBuilder();
            • for (int i = 0; i < expression.length(); i++) {
            • char ch = expression.charAt;
            • sb.append(isReservedCharacter(ch) ? "
              " + ch : ch);
              + int length = expression.length();

            Review comment:
            This being micro-optimization or not should really be supported by benchmark result data. If this is on a hot path, creating less StringBuilder can really make some differences.
            This said, please note that the fix actually solves 2 issue:
            1) it avoids creating the StringBuilder when not needed
            2) it prevents useless StringBuilder instances being created whenever a reserved character is found, as sb.append(isReservedCharacter(ch) ? "
            " + ch : ch) internally creates another StringBuilder to perform the "
            " + ch concatenation.

            ----------------------------------------------------------------
            This is an automated message from the Apache Git Service.
            To respond to the message, please log on GitHub and use the
            URL above to go to the specific comment.

            For queries about this service, please contact Infrastructure at:
            users@infra.apache.org

            githubbot ASF GitHub Bot added a comment - asoldano commented on a change in pull request #407: CXF-7716 Reduce StringBuilders and other performance changes. URL: https://github.com/apache/cxf/pull/407#discussion_r182964151 ########## File path: rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/model/URITemplate.java ########## @@ -130,10 +130,31 @@ public String getPatternValue() { private static String escapeCharacters(String expression) { StringBuilder sb = new StringBuilder(); for (int i = 0; i < expression.length(); i++) { char ch = expression.charAt ; sb.append(isReservedCharacter(ch) ? " " + ch : ch); + int length = expression.length(); Review comment: This being micro-optimization or not should really be supported by benchmark result data. If this is on a hot path, creating less StringBuilder can really make some differences. This said, please note that the fix actually solves 2 issue: 1) it avoids creating the StringBuilder when not needed 2) it prevents useless StringBuilder instances being created whenever a reserved character is found, as sb.append(isReservedCharacter(ch) ? " " + ch : ch) internally creates another StringBuilder to perform the " " + ch concatenation. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: users@infra.apache.org
            githubbot ASF GitHub Bot added a comment -

            WhiteCat22 commented on a change in pull request #407: CXF-7716 Reduce StringBuilders and other performance changes.
            URL: https://github.com/apache/cxf/pull/407#discussion_r183068721

            ##########
            File path: rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/model/URITemplate.java
            ##########
            @@ -130,10 +130,31 @@ public String getPatternValue() {

            private static String escapeCharacters(String expression) {

            • StringBuilder sb = new StringBuilder();
            • for (int i = 0; i < expression.length(); i++) {
            • char ch = expression.charAt;
            • sb.append(isReservedCharacter(ch) ? "
              " + ch : ch);
              + int length = expression.length();

            Review comment:
            We don't have hard numbers at the moment, but all of these changes combined are estimated to improve performance by 1.5%-2% per request.

            Our JDK team analyzed our performance in order to reduce CPU usage and garbage collection and found that we are creating a large number of StringBuilders. StringBuilders are expensive to use so only using them when we need them will improve performance. Similarly, they found that we were using reflection to get the params every request in ResourceUtils.createConstructorArguments() which is also expensive, hence the change to get the params one time in the constructor.

            ----------------------------------------------------------------
            This is an automated message from the Apache Git Service.
            To respond to the message, please log on GitHub and use the
            URL above to go to the specific comment.

            For queries about this service, please contact Infrastructure at:
            users@infra.apache.org

            githubbot ASF GitHub Bot added a comment - WhiteCat22 commented on a change in pull request #407: CXF-7716 Reduce StringBuilders and other performance changes. URL: https://github.com/apache/cxf/pull/407#discussion_r183068721 ########## File path: rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/model/URITemplate.java ########## @@ -130,10 +130,31 @@ public String getPatternValue() { private static String escapeCharacters(String expression) { StringBuilder sb = new StringBuilder(); for (int i = 0; i < expression.length(); i++) { char ch = expression.charAt ; sb.append(isReservedCharacter(ch) ? " " + ch : ch); + int length = expression.length(); Review comment: We don't have hard numbers at the moment, but all of these changes combined are estimated to improve performance by 1.5%-2% per request. Our JDK team analyzed our performance in order to reduce CPU usage and garbage collection and found that we are creating a large number of StringBuilders. StringBuilders are expensive to use so only using them when we need them will improve performance. Similarly, they found that we were using reflection to get the params every request in ResourceUtils.createConstructorArguments() which is also expensive, hence the change to get the params one time in the constructor. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: users@infra.apache.org
            githubbot ASF GitHub Bot added a comment -

            WhiteCat22 commented on a change in pull request #407: CXF-7716 Reduce StringBuilders and other performance changes.
            URL: https://github.com/apache/cxf/pull/407#discussion_r183085465

            ##########
            File path: rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/model/URITemplate.java
            ##########
            @@ -130,10 +130,31 @@ public String getPatternValue() {

            private static String escapeCharacters(String expression) {

            • StringBuilder sb = new StringBuilder();
            • for (int i = 0; i < expression.length(); i++) {
            • char ch = expression.charAt;
            • sb.append(isReservedCharacter(ch) ? "
              " + ch : ch);
              + int length = expression.length();
              + int i = 0;
              + char ch = ' ';
              + for (; i < length; ++i)
              Unknown macro: {+ ch = expression.charAt(i);+ if (isReservedCharacter(ch)) { + break; + }+ }

              +
              + if (i == length)

              { + return expression; + }

              +
              + StringBuilder sb = new StringBuilder(length + 8);

            Review comment:
            We do + 8 to allows for up to 8 escaped characters before we start creating more StringBuilders to add on the the String. 8 is an arbitrary limit, but it seems to be sufficient in most cases. We could do something like 256, but that would be overkill since it's rare that someone would actually have 256 escaped characters.

            ----------------------------------------------------------------
            This is an automated message from the Apache Git Service.
            To respond to the message, please log on GitHub and use the
            URL above to go to the specific comment.

            For queries about this service, please contact Infrastructure at:
            users@infra.apache.org

            githubbot ASF GitHub Bot added a comment - WhiteCat22 commented on a change in pull request #407: CXF-7716 Reduce StringBuilders and other performance changes. URL: https://github.com/apache/cxf/pull/407#discussion_r183085465 ########## File path: rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/model/URITemplate.java ########## @@ -130,10 +130,31 @@ public String getPatternValue() { private static String escapeCharacters(String expression) { StringBuilder sb = new StringBuilder(); for (int i = 0; i < expression.length(); i++) { char ch = expression.charAt ; sb.append(isReservedCharacter(ch) ? " " + ch : ch); + int length = expression.length(); + int i = 0; + char ch = ' '; + for (; i < length; ++i) Unknown macro: {+ ch = expression.charAt(i);+ if (isReservedCharacter(ch)) { + break; + }+ } + + if (i == length) { + return expression; + } + + StringBuilder sb = new StringBuilder(length + 8); Review comment: We do + 8 to allows for up to 8 escaped characters before we start creating more StringBuilders to add on the the String. 8 is an arbitrary limit, but it seems to be sufficient in most cases. We could do something like 256, but that would be overkill since it's rare that someone would actually have 256 escaped characters. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: users@infra.apache.org
            githubbot ASF GitHub Bot added a comment -

            WhiteCat22 commented on a change in pull request #407: CXF-7716 Reduce StringBuilders and other performance changes.
            URL: https://github.com/apache/cxf/pull/407#discussion_r183068721

            ##########
            File path: rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/model/URITemplate.java
            ##########
            @@ -130,10 +130,31 @@ public String getPatternValue() {

            private static String escapeCharacters(String expression) {

            • StringBuilder sb = new StringBuilder();
            • for (int i = 0; i < expression.length(); i++) {
            • char ch = expression.charAt;
            • sb.append(isReservedCharacter(ch) ? "
              " + ch : ch);
              + int length = expression.length();

            Review comment:
            We don't have hard numbers at the moment, but all of these changes combined are estimated to improve performance by 1.5%-2% per request depending on load.

            Our JDK team analyzed our performance in order to reduce CPU usage and garbage collection and found that we are creating a large number of StringBuilders. StringBuilders are expensive to use so only using them when we need them will improve performance. Similarly, they found that we were using reflection to get the params every request in ResourceUtils.createConstructorArguments() which is also expensive, hence the change to get the params one time in the constructor.

            ----------------------------------------------------------------
            This is an automated message from the Apache Git Service.
            To respond to the message, please log on GitHub and use the
            URL above to go to the specific comment.

            For queries about this service, please contact Infrastructure at:
            users@infra.apache.org

            githubbot ASF GitHub Bot added a comment - WhiteCat22 commented on a change in pull request #407: CXF-7716 Reduce StringBuilders and other performance changes. URL: https://github.com/apache/cxf/pull/407#discussion_r183068721 ########## File path: rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/model/URITemplate.java ########## @@ -130,10 +130,31 @@ public String getPatternValue() { private static String escapeCharacters(String expression) { StringBuilder sb = new StringBuilder(); for (int i = 0; i < expression.length(); i++) { char ch = expression.charAt ; sb.append(isReservedCharacter(ch) ? " " + ch : ch); + int length = expression.length(); Review comment: We don't have hard numbers at the moment, but all of these changes combined are estimated to improve performance by 1.5%-2% per request depending on load. Our JDK team analyzed our performance in order to reduce CPU usage and garbage collection and found that we are creating a large number of StringBuilders. StringBuilders are expensive to use so only using them when we need them will improve performance. Similarly, they found that we were using reflection to get the params every request in ResourceUtils.createConstructorArguments() which is also expensive, hence the change to get the params one time in the constructor. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: users@infra.apache.org
            githubbot ASF GitHub Bot added a comment -

            coheigea commented on a change in pull request #407: CXF-7716 Reduce StringBuilders and other performance changes.
            URL: https://github.com/apache/cxf/pull/407#discussion_r183098421

            ##########
            File path: rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/model/URITemplate.java
            ##########
            @@ -130,10 +130,31 @@ public String getPatternValue() {

            private static String escapeCharacters(String expression) {

            • StringBuilder sb = new StringBuilder();
            • for (int i = 0; i < expression.length(); i++) {
            • char ch = expression.charAt;
            • sb.append(isReservedCharacter(ch) ? "
              " + ch : ch);
              + int length = expression.length();
              + int i = 0;
              + char ch = ' ';
              + for (; i < length; ++i)
              Unknown macro: {+ ch = expression.charAt(i);+ if (isReservedCharacter(ch)) { + break; + }+ }

              +
              + if (i == length)

              { + return expression; + }

              +
              + StringBuilder sb = new StringBuilder(length + 8);

            Review comment:
            There should be a comment in the code in that case explaining why "8" is chosen

            ----------------------------------------------------------------
            This is an automated message from the Apache Git Service.
            To respond to the message, please log on GitHub and use the
            URL above to go to the specific comment.

            For queries about this service, please contact Infrastructure at:
            users@infra.apache.org

            githubbot ASF GitHub Bot added a comment - coheigea commented on a change in pull request #407: CXF-7716 Reduce StringBuilders and other performance changes. URL: https://github.com/apache/cxf/pull/407#discussion_r183098421 ########## File path: rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/model/URITemplate.java ########## @@ -130,10 +130,31 @@ public String getPatternValue() { private static String escapeCharacters(String expression) { StringBuilder sb = new StringBuilder(); for (int i = 0; i < expression.length(); i++) { char ch = expression.charAt ; sb.append(isReservedCharacter(ch) ? " " + ch : ch); + int length = expression.length(); + int i = 0; + char ch = ' '; + for (; i < length; ++i) Unknown macro: {+ ch = expression.charAt(i);+ if (isReservedCharacter(ch)) { + break; + }+ } + + if (i == length) { + return expression; + } + + StringBuilder sb = new StringBuilder(length + 8); Review comment: There should be a comment in the code in that case explaining why "8" is chosen ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: users@infra.apache.org
            githubbot ASF GitHub Bot added a comment -

            WhiteCat22 commented on a change in pull request #407: CXF-7716 Reduce StringBuilders and other performance changes.
            URL: https://github.com/apache/cxf/pull/407#discussion_r183100561

            ##########
            File path: rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/model/URITemplate.java
            ##########
            @@ -130,10 +130,34 @@ public String getPatternValue() {

            private static String escapeCharacters(String expression) {

            • StringBuilder sb = new StringBuilder();
            • for (int i = 0; i < expression.length(); i++) {
            • char ch = expression.charAt;
            • sb.append(isReservedCharacter(ch) ? "
              " + ch : ch);
              + int length = expression.length();
              + int i = 0;
              + char ch = ' ';
              + for (; i < length; ++i)
              Unknown macro: {+ ch = expression.charAt(i);+ if (isReservedCharacter(ch)) { + break; + }+ }

              +
              + if (i == length)

              { + return expression; + }

              +
              + // Allows for up to 8 escaped characters before we start creating more
              + // StringBuilders. 8 is an arbitrary limit, but it seems to be
              + // sufficient in most cases.
              + StringBuilder sb = new StringBuilder(length + 8);

            Review comment:
            Apparently amending the file removes comments, but I added a comment explaining why we add +8 to the length.

            ----------------------------------------------------------------
            This is an automated message from the Apache Git Service.
            To respond to the message, please log on GitHub and use the
            URL above to go to the specific comment.

            For queries about this service, please contact Infrastructure at:
            users@infra.apache.org

            githubbot ASF GitHub Bot added a comment - WhiteCat22 commented on a change in pull request #407: CXF-7716 Reduce StringBuilders and other performance changes. URL: https://github.com/apache/cxf/pull/407#discussion_r183100561 ########## File path: rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/model/URITemplate.java ########## @@ -130,10 +130,34 @@ public String getPatternValue() { private static String escapeCharacters(String expression) { StringBuilder sb = new StringBuilder(); for (int i = 0; i < expression.length(); i++) { char ch = expression.charAt ; sb.append(isReservedCharacter(ch) ? " " + ch : ch); + int length = expression.length(); + int i = 0; + char ch = ' '; + for (; i < length; ++i) Unknown macro: {+ ch = expression.charAt(i);+ if (isReservedCharacter(ch)) { + break; + }+ } + + if (i == length) { + return expression; + } + + // Allows for up to 8 escaped characters before we start creating more + // StringBuilders. 8 is an arbitrary limit, but it seems to be + // sufficient in most cases. + StringBuilder sb = new StringBuilder(length + 8); Review comment: Apparently amending the file removes comments, but I added a comment explaining why we add +8 to the length. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: users@infra.apache.org
            githubbot ASF GitHub Bot added a comment -

            reta commented on a change in pull request #407: CXF-7716 Reduce StringBuilders and other performance changes.
            URL: https://github.com/apache/cxf/pull/407#discussion_r183259135

            ##########
            File path: rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/JAXRSUtils.java
            ##########
            @@ -1508,11 +1509,40 @@ public static boolean matchMimeTypes(MediaType requestContentType,
            }
            }
            }
            -
            return new ArrayList<>(supportedMimeTypeList);
            + }
            +
            + public static boolean doMimeTypesIntersect(List<MediaType> requiredMediaTypes, List<MediaType> userMediaTypes) {
            + for (MediaType requiredType : requiredMediaTypes) {

            Review comment:
            Regarding this part, I understand the purpose, but now there 2 identical places, `intersectMimeTypes` and `doMimeTypesIntersect`, with the same code (solving one problem but introducing another). What I am thinking about is to introduce some kind of accumulator to either map the result of the operation to the `List` or `true/false`, do you think it would be possible @WhiteCat22 ?

            ----------------------------------------------------------------
            This is an automated message from the Apache Git Service.
            To respond to the message, please log on GitHub and use the
            URL above to go to the specific comment.

            For queries about this service, please contact Infrastructure at:
            users@infra.apache.org

            githubbot ASF GitHub Bot added a comment - reta commented on a change in pull request #407: CXF-7716 Reduce StringBuilders and other performance changes. URL: https://github.com/apache/cxf/pull/407#discussion_r183259135 ########## File path: rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/JAXRSUtils.java ########## @@ -1508,11 +1509,40 @@ public static boolean matchMimeTypes(MediaType requestContentType, } } } - return new ArrayList<>(supportedMimeTypeList); + } + + public static boolean doMimeTypesIntersect(List<MediaType> requiredMediaTypes, List<MediaType> userMediaTypes) { + for (MediaType requiredType : requiredMediaTypes) { Review comment: Regarding this part, I understand the purpose, but now there 2 identical places, `intersectMimeTypes` and `doMimeTypesIntersect`, with the same code (solving one problem but introducing another). What I am thinking about is to introduce some kind of accumulator to either map the result of the operation to the `List` or `true/false`, do you think it would be possible @WhiteCat22 ? ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: users@infra.apache.org
            githubbot ASF GitHub Bot added a comment -

            WhiteCat22 commented on a change in pull request #407: CXF-7716 Reduce StringBuilders and other performance changes.
            URL: https://github.com/apache/cxf/pull/407#discussion_r183534925

            ##########
            File path: rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/JAXRSUtils.java
            ##########
            @@ -1508,11 +1509,40 @@ public static boolean matchMimeTypes(MediaType requestContentType,
            }
            }
            }
            -
            return new ArrayList<>(supportedMimeTypeList);
            + }
            +
            + public static boolean doMimeTypesIntersect(List<MediaType> requiredMediaTypes, List<MediaType> userMediaTypes) {
            + for (MediaType requiredType : requiredMediaTypes) {

            Review comment:
            The performance increase comes from not making a HashSet just to do the comparison. I agree that duplicating code is not ideal, but I'm not sure how to accomplish both cases with a single method.

            I am open to suggestions.

            ----------------------------------------------------------------
            This is an automated message from the Apache Git Service.
            To respond to the message, please log on GitHub and use the
            URL above to go to the specific comment.

            For queries about this service, please contact Infrastructure at:
            users@infra.apache.org

            githubbot ASF GitHub Bot added a comment - WhiteCat22 commented on a change in pull request #407: CXF-7716 Reduce StringBuilders and other performance changes. URL: https://github.com/apache/cxf/pull/407#discussion_r183534925 ########## File path: rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/JAXRSUtils.java ########## @@ -1508,11 +1509,40 @@ public static boolean matchMimeTypes(MediaType requestContentType, } } } - return new ArrayList<>(supportedMimeTypeList); + } + + public static boolean doMimeTypesIntersect(List<MediaType> requiredMediaTypes, List<MediaType> userMediaTypes) { + for (MediaType requiredType : requiredMediaTypes) { Review comment: The performance increase comes from not making a HashSet just to do the comparison. I agree that duplicating code is not ideal, but I'm not sure how to accomplish both cases with a single method. I am open to suggestions. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: users@infra.apache.org
            githubbot ASF GitHub Bot added a comment -

            reta commented on a change in pull request #407: CXF-7716 Reduce StringBuilders and other performance changes.
            URL: https://github.com/apache/cxf/pull/407#discussion_r183578479

            ##########
            File path: rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/JAXRSUtils.java
            ##########
            @@ -1508,11 +1509,40 @@ public static boolean matchMimeTypes(MediaType requestContentType,
            }
            }
            }
            -
            return new ArrayList<>(supportedMimeTypeList);
            + }
            +
            + public static boolean doMimeTypesIntersect(List<MediaType> requiredMediaTypes, List<MediaType> userMediaTypes) {
            + for (MediaType requiredType : requiredMediaTypes) {

            Review comment:
            Turned out to be quite tricky ... but I have one suggestion for you. Here is the implementation for `doMimeTypesIntersect` and `intersectMimeTypes`:

            ```
            public static List<MediaType> intersectMimeTypes(List<MediaType> requiredMediaTypes,
            List<MediaType> userMediaTypes,
            boolean addRequiredParamsIfPossible,
            boolean addDistanceParameter)

            { final AccumulatingConsumer consumer = new AccumulatingConsumer(addRequiredParamsIfPossible, addDistanceParameter); intersectMimeTypes(requiredMediaTypes, userMediaTypes, consumer); return new ArrayList<>(consumer.supportedMimeTypeList); }

            public static boolean doMimeTypesIntersect(List<MediaType> requiredMediaTypes, List<MediaType> userMediaTypes)

            { final NonAccumulatingConsumer consumer = new NonAccumulatingConsumer(); intersectMimeTypes(requiredMediaTypes, userMediaTypes, consumer); return consumer.doIntersect; }

            ```
            Here is the initial `intersectMimeTypes` function, refactored to use consumers, the part which is was accumulating (or returning `true`) is now part of the consumers:

            ```
            private static void intersectMimeTypes(List<MediaType> requiredMediaTypes, List<MediaType> userMediaTypes,
            BiConsumer<MediaType, MediaType> consumer) {

            for (MediaType requiredType : requiredMediaTypes) {
            for (MediaType userType : userMediaTypes) {
            boolean isCompatible = isMediaTypeCompatible(requiredType, userType);
            if (isCompatible) {
            boolean parametersMatched = true;
            for (Map.Entry<String, String> entry : userType.getParameters().entrySet()) {
            String value = requiredType.getParameters().get(entry.getKey());
            if (value != null && entry.getValue() != null
            && !(stripDoubleQuotesIfNeeded(value).equals(
            stripDoubleQuotesIfNeeded(entry.getValue())))) {

            if (HTTP_CHARSET_PARAM.equals(entry.getKey())
            && value.equalsIgnoreCase(entry.getValue()))

            { continue; }

            parametersMatched = false;
            break;
            }
            }
            if (!parametersMatched)

            { continue; }

            consumer.accept(requiredType, userType);
            }
            }
            }
            }

            ```

            And finally, two consumers:
            ```
            private static class NonAccumulatingConsumer implements BiConsumer<MediaType, MediaType> {
            private boolean doIntersect;

            @Override
            public void accept(MediaType t, MediaType u)

            { doIntersect = true; }

            }
            ```

            And
            ```
            private static class AccumulatingConsumer implements BiConsumer<MediaType, MediaType> {
            private final Set<MediaType> supportedMimeTypeList = new LinkedHashSet<MediaType>();
            private final boolean addRequiredParamsIfPossible;
            private final boolean addDistanceParameter;

            private AccumulatingConsumer(boolean addRequiredParamsIfPossible, boolean addDistanceParameter)

            { this.addRequiredParamsIfPossible = addRequiredParamsIfPossible; this.addDistanceParameter = addDistanceParameter; }

            @Override
            public void accept(MediaType requiredType, MediaType userType) {
            boolean requiredTypeWildcard = requiredType.getType().equals(MediaType.MEDIA_TYPE_WILDCARD);
            boolean requiredSubTypeWildcard = requiredType.getSubtype().contains(MediaType.MEDIA_TYPE_WILDCARD);

            String type = requiredTypeWildcard ? userType.getType() : requiredType.getType();
            String subtype = requiredSubTypeWildcard ? userType.getSubtype() : requiredType.getSubtype();

            Map<String, String> parameters = userType.getParameters();
            if (addRequiredParamsIfPossible) {
            parameters = new LinkedHashMap<String, String>(parameters);
            for (Map.Entry<String, String> entry : requiredType.getParameters().entrySet()) {
            if (!parameters.containsKey(entry.getKey()))

            { parameters.put(entry.getKey(), entry.getValue()); }

            }
            }
            if (addDistanceParameter) {
            int distance = 0;
            if (requiredTypeWildcard)

            { distance++; }
            if (requiredSubTypeWildcard) { distance++; }

            parameters.put(MEDIA_TYPE_DISTANCE_PARAM, Integer.toString(distance));
            }
            supportedMimeTypeList.add(new MediaType(type, subtype, parameters));
            }
            }
            ```

            Ideally, would be good to move to a dedicated utility class, but roughly this is an idea. What do you think. @WhiteCat22, does it make sense?

            ----------------------------------------------------------------
            This is an automated message from the Apache Git Service.
            To respond to the message, please log on GitHub and use the
            URL above to go to the specific comment.

            For queries about this service, please contact Infrastructure at:
            users@infra.apache.org

            githubbot ASF GitHub Bot added a comment - reta commented on a change in pull request #407: CXF-7716 Reduce StringBuilders and other performance changes. URL: https://github.com/apache/cxf/pull/407#discussion_r183578479 ########## File path: rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/JAXRSUtils.java ########## @@ -1508,11 +1509,40 @@ public static boolean matchMimeTypes(MediaType requestContentType, } } } - return new ArrayList<>(supportedMimeTypeList); + } + + public static boolean doMimeTypesIntersect(List<MediaType> requiredMediaTypes, List<MediaType> userMediaTypes) { + for (MediaType requiredType : requiredMediaTypes) { Review comment: Turned out to be quite tricky ... but I have one suggestion for you. Here is the implementation for `doMimeTypesIntersect` and `intersectMimeTypes`: ``` public static List<MediaType> intersectMimeTypes(List<MediaType> requiredMediaTypes, List<MediaType> userMediaTypes, boolean addRequiredParamsIfPossible, boolean addDistanceParameter) { final AccumulatingConsumer consumer = new AccumulatingConsumer(addRequiredParamsIfPossible, addDistanceParameter); intersectMimeTypes(requiredMediaTypes, userMediaTypes, consumer); return new ArrayList<>(consumer.supportedMimeTypeList); } public static boolean doMimeTypesIntersect(List<MediaType> requiredMediaTypes, List<MediaType> userMediaTypes) { final NonAccumulatingConsumer consumer = new NonAccumulatingConsumer(); intersectMimeTypes(requiredMediaTypes, userMediaTypes, consumer); return consumer.doIntersect; } ``` Here is the initial `intersectMimeTypes` function, refactored to use consumers, the part which is was accumulating (or returning `true`) is now part of the consumers: ``` private static void intersectMimeTypes(List<MediaType> requiredMediaTypes, List<MediaType> userMediaTypes, BiConsumer<MediaType, MediaType> consumer) { for (MediaType requiredType : requiredMediaTypes) { for (MediaType userType : userMediaTypes) { boolean isCompatible = isMediaTypeCompatible(requiredType, userType); if (isCompatible) { boolean parametersMatched = true; for (Map.Entry<String, String> entry : userType.getParameters().entrySet()) { String value = requiredType.getParameters().get(entry.getKey()); if (value != null && entry.getValue() != null && !(stripDoubleQuotesIfNeeded(value).equals( stripDoubleQuotesIfNeeded(entry.getValue())))) { if (HTTP_CHARSET_PARAM.equals(entry.getKey()) && value.equalsIgnoreCase(entry.getValue())) { continue; } parametersMatched = false; break; } } if (!parametersMatched) { continue; } consumer.accept(requiredType, userType); } } } } ``` And finally, two consumers: ``` private static class NonAccumulatingConsumer implements BiConsumer<MediaType, MediaType> { private boolean doIntersect; @Override public void accept(MediaType t, MediaType u) { doIntersect = true; } } ``` And ``` private static class AccumulatingConsumer implements BiConsumer<MediaType, MediaType> { private final Set<MediaType> supportedMimeTypeList = new LinkedHashSet<MediaType>(); private final boolean addRequiredParamsIfPossible; private final boolean addDistanceParameter; private AccumulatingConsumer(boolean addRequiredParamsIfPossible, boolean addDistanceParameter) { this.addRequiredParamsIfPossible = addRequiredParamsIfPossible; this.addDistanceParameter = addDistanceParameter; } @Override public void accept(MediaType requiredType, MediaType userType) { boolean requiredTypeWildcard = requiredType.getType().equals(MediaType.MEDIA_TYPE_WILDCARD); boolean requiredSubTypeWildcard = requiredType.getSubtype().contains(MediaType.MEDIA_TYPE_WILDCARD); String type = requiredTypeWildcard ? userType.getType() : requiredType.getType(); String subtype = requiredSubTypeWildcard ? userType.getSubtype() : requiredType.getSubtype(); Map<String, String> parameters = userType.getParameters(); if (addRequiredParamsIfPossible) { parameters = new LinkedHashMap<String, String>(parameters); for (Map.Entry<String, String> entry : requiredType.getParameters().entrySet()) { if (!parameters.containsKey(entry.getKey())) { parameters.put(entry.getKey(), entry.getValue()); } } } if (addDistanceParameter) { int distance = 0; if (requiredTypeWildcard) { distance++; } if (requiredSubTypeWildcard) { distance++; } parameters.put(MEDIA_TYPE_DISTANCE_PARAM, Integer.toString(distance)); } supportedMimeTypeList.add(new MediaType(type, subtype, parameters)); } } ``` Ideally, would be good to move to a dedicated utility class, but roughly this is an idea. What do you think. @WhiteCat22, does it make sense? ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: users@infra.apache.org
            githubbot ASF GitHub Bot added a comment -

            WhiteCat22 commented on a change in pull request #407: CXF-7716 Reduce StringBuilders and other performance changes.
            URL: https://github.com/apache/cxf/pull/407#discussion_r183899002

            ##########
            File path: rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/JAXRSUtils.java
            ##########
            @@ -1508,11 +1509,40 @@ public static boolean matchMimeTypes(MediaType requestContentType,
            }
            }
            }
            -
            return new ArrayList<>(supportedMimeTypeList);
            + }
            +
            + public static boolean doMimeTypesIntersect(List<MediaType> requiredMediaTypes, List<MediaType> userMediaTypes) {
            + for (MediaType requiredType : requiredMediaTypes) {

            Review comment:
            This makes sense, but is there another interface that we can use instead of BiConsumer? BiConsumer requires Java 1.8 and this is something that we would like to see backported to 3.1.X which has 1.7 dependencies. Another thing is that we need to have a mechanism to return after the first match during NonAccumulatingConsumer .accept() instead of going through the entire double for loops. There are several ways to do this, pass in a flag to allow us to bail out after first match, return a boolean from accept, or create a new isFinished() method to let us know that we are done processing.

            ----------------------------------------------------------------
            This is an automated message from the Apache Git Service.
            To respond to the message, please log on GitHub and use the
            URL above to go to the specific comment.

            For queries about this service, please contact Infrastructure at:
            users@infra.apache.org

            githubbot ASF GitHub Bot added a comment - WhiteCat22 commented on a change in pull request #407: CXF-7716 Reduce StringBuilders and other performance changes. URL: https://github.com/apache/cxf/pull/407#discussion_r183899002 ########## File path: rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/JAXRSUtils.java ########## @@ -1508,11 +1509,40 @@ public static boolean matchMimeTypes(MediaType requestContentType, } } } - return new ArrayList<>(supportedMimeTypeList); + } + + public static boolean doMimeTypesIntersect(List<MediaType> requiredMediaTypes, List<MediaType> userMediaTypes) { + for (MediaType requiredType : requiredMediaTypes) { Review comment: This makes sense, but is there another interface that we can use instead of BiConsumer? BiConsumer requires Java 1.8 and this is something that we would like to see backported to 3.1.X which has 1.7 dependencies. Another thing is that we need to have a mechanism to return after the first match during NonAccumulatingConsumer .accept() instead of going through the entire double for loops. There are several ways to do this, pass in a flag to allow us to bail out after first match, return a boolean from accept, or create a new isFinished() method to let us know that we are done processing. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: users@infra.apache.org
            githubbot ASF GitHub Bot added a comment -

            reta commented on a change in pull request #407: CXF-7716 Reduce StringBuilders and other performance changes.
            URL: https://github.com/apache/cxf/pull/407#discussion_r183927574

            ##########
            File path: rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/JAXRSUtils.java
            ##########
            @@ -1508,11 +1509,40 @@ public static boolean matchMimeTypes(MediaType requestContentType,
            }
            }
            }
            -
            return new ArrayList<>(supportedMimeTypeList);
            + }
            +
            + public static boolean doMimeTypesIntersect(List<MediaType> requiredMediaTypes, List<MediaType> userMediaTypes) {
            + for (MediaType requiredType : requiredMediaTypes) {

            Review comment:
            Yeah, sure, sounds good, something like that would fit:
            ```
            public interface MimeTypesIntersector

            { boolean void intersect(MediaType requiredType, MediaType userType); }

            ```

            And than

            ```
            private static void intersectMimeTypes(List<MediaType> requiredMediaTypes, List<MediaType> userMediaTypes,
            MimeTypesIntersector intersector) {
            ...
            if (!intersector.intersect( requiredType, userType)

            { return; }

            }
            ```

            ----------------------------------------------------------------
            This is an automated message from the Apache Git Service.
            To respond to the message, please log on GitHub and use the
            URL above to go to the specific comment.

            For queries about this service, please contact Infrastructure at:
            users@infra.apache.org

            githubbot ASF GitHub Bot added a comment - reta commented on a change in pull request #407: CXF-7716 Reduce StringBuilders and other performance changes. URL: https://github.com/apache/cxf/pull/407#discussion_r183927574 ########## File path: rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/JAXRSUtils.java ########## @@ -1508,11 +1509,40 @@ public static boolean matchMimeTypes(MediaType requestContentType, } } } - return new ArrayList<>(supportedMimeTypeList); + } + + public static boolean doMimeTypesIntersect(List<MediaType> requiredMediaTypes, List<MediaType> userMediaTypes) { + for (MediaType requiredType : requiredMediaTypes) { Review comment: Yeah, sure, sounds good, something like that would fit: ``` public interface MimeTypesIntersector { boolean void intersect(MediaType requiredType, MediaType userType); } ``` And than ``` private static void intersectMimeTypes(List<MediaType> requiredMediaTypes, List<MediaType> userMediaTypes, MimeTypesIntersector intersector) { ... if (!intersector.intersect( requiredType, userType) { return; } } ``` ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: users@infra.apache.org
            githubbot ASF GitHub Bot added a comment -

            WhiteCat22 commented on a change in pull request #407: CXF-7716 Reduce StringBuilders and other performance changes.
            URL: https://github.com/apache/cxf/pull/407#discussion_r186559564

            ##########
            File path: rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/JAXRSUtils.java
            ##########
            @@ -1508,11 +1509,40 @@ public static boolean matchMimeTypes(MediaType requestContentType,
            }
            }
            }
            -
            return new ArrayList<>(supportedMimeTypeList);
            + }
            +
            + public static boolean doMimeTypesIntersect(List<MediaType> requiredMediaTypes, List<MediaType> userMediaTypes) {
            + for (MediaType requiredType : requiredMediaTypes) {

            Review comment:
            Alright, I've had a chance to code this up. Should these classes each go in their own class in the `org.apache.cxf.jaxrs.utils` package, or a single util class?

            ----------------------------------------------------------------
            This is an automated message from the Apache Git Service.
            To respond to the message, please log on GitHub and use the
            URL above to go to the specific comment.

            For queries about this service, please contact Infrastructure at:
            users@infra.apache.org

            githubbot ASF GitHub Bot added a comment - WhiteCat22 commented on a change in pull request #407: CXF-7716 Reduce StringBuilders and other performance changes. URL: https://github.com/apache/cxf/pull/407#discussion_r186559564 ########## File path: rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/JAXRSUtils.java ########## @@ -1508,11 +1509,40 @@ public static boolean matchMimeTypes(MediaType requestContentType, } } } - return new ArrayList<>(supportedMimeTypeList); + } + + public static boolean doMimeTypesIntersect(List<MediaType> requiredMediaTypes, List<MediaType> userMediaTypes) { + for (MediaType requiredType : requiredMediaTypes) { Review comment: Alright, I've had a chance to code this up. Should these classes each go in their own class in the `org.apache.cxf.jaxrs.utils` package, or a single util class? ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: users@infra.apache.org
            githubbot ASF GitHub Bot added a comment -

            reta commented on a change in pull request #407: CXF-7716 Reduce StringBuilders and other performance changes.
            URL: https://github.com/apache/cxf/pull/407#discussion_r186562512

            ##########
            File path: rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/JAXRSUtils.java
            ##########
            @@ -1508,11 +1509,40 @@ public static boolean matchMimeTypes(MediaType requestContentType,
            }
            }
            }
            -
            return new ArrayList<>(supportedMimeTypeList);
            + }
            +
            + public static boolean doMimeTypesIntersect(List<MediaType> requiredMediaTypes, List<MediaType> userMediaTypes) {
            + for (MediaType requiredType : requiredMediaTypes) {

            Review comment:
            I would vote for own class, what do you think?

            ----------------------------------------------------------------
            This is an automated message from the Apache Git Service.
            To respond to the message, please log on GitHub and use the
            URL above to go to the specific comment.

            For queries about this service, please contact Infrastructure at:
            users@infra.apache.org

            githubbot ASF GitHub Bot added a comment - reta commented on a change in pull request #407: CXF-7716 Reduce StringBuilders and other performance changes. URL: https://github.com/apache/cxf/pull/407#discussion_r186562512 ########## File path: rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/JAXRSUtils.java ########## @@ -1508,11 +1509,40 @@ public static boolean matchMimeTypes(MediaType requestContentType, } } } - return new ArrayList<>(supportedMimeTypeList); + } + + public static boolean doMimeTypesIntersect(List<MediaType> requiredMediaTypes, List<MediaType> userMediaTypes) { + for (MediaType requiredType : requiredMediaTypes) { Review comment: I would vote for own class, what do you think? ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: users@infra.apache.org
            githubbot ASF GitHub Bot added a comment -

            WhiteCat22 commented on a change in pull request #407: CXF-7716 Reduce StringBuilders and other performance changes.
            URL: https://github.com/apache/cxf/pull/407#discussion_r186566722

            ##########
            File path: rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/JAXRSUtils.java
            ##########
            @@ -1508,11 +1509,40 @@ public static boolean matchMimeTypes(MediaType requestContentType,
            }
            }
            }
            -
            return new ArrayList<>(supportedMimeTypeList);
            + }
            +
            + public static boolean doMimeTypesIntersect(List<MediaType> requiredMediaTypes, List<MediaType> userMediaTypes) {
            + for (MediaType requiredType : requiredMediaTypes) {

            Review comment:
            I agree, I have them coded up into separate `MimeTypesIntersector`, `AccumulatingIntersector` and `NonAccumulatingIntersector` classes

            ----------------------------------------------------------------
            This is an automated message from the Apache Git Service.
            To respond to the message, please log on GitHub and use the
            URL above to go to the specific comment.

            For queries about this service, please contact Infrastructure at:
            users@infra.apache.org

            githubbot ASF GitHub Bot added a comment - WhiteCat22 commented on a change in pull request #407: CXF-7716 Reduce StringBuilders and other performance changes. URL: https://github.com/apache/cxf/pull/407#discussion_r186566722 ########## File path: rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/JAXRSUtils.java ########## @@ -1508,11 +1509,40 @@ public static boolean matchMimeTypes(MediaType requestContentType, } } } - return new ArrayList<>(supportedMimeTypeList); + } + + public static boolean doMimeTypesIntersect(List<MediaType> requiredMediaTypes, List<MediaType> userMediaTypes) { + for (MediaType requiredType : requiredMediaTypes) { Review comment: I agree, I have them coded up into separate `MimeTypesIntersector`, `AccumulatingIntersector` and `NonAccumulatingIntersector` classes ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: users@infra.apache.org
            githubbot ASF GitHub Bot added a comment -

            reta commented on a change in pull request #407: CXF-7716 Reduce StringBuilders and other performance changes.
            URL: https://github.com/apache/cxf/pull/407#discussion_r186569414

            ##########
            File path: rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/JAXRSUtils.java
            ##########
            @@ -1508,11 +1509,40 @@ public static boolean matchMimeTypes(MediaType requestContentType,
            }
            }
            }
            -
            return new ArrayList<>(supportedMimeTypeList);
            + }
            +
            + public static boolean doMimeTypesIntersect(List<MediaType> requiredMediaTypes, List<MediaType> userMediaTypes) {
            + for (MediaType requiredType : requiredMediaTypes) {

            Review comment:
            :+1:

            ----------------------------------------------------------------
            This is an automated message from the Apache Git Service.
            To respond to the message, please log on GitHub and use the
            URL above to go to the specific comment.

            For queries about this service, please contact Infrastructure at:
            users@infra.apache.org

            githubbot ASF GitHub Bot added a comment - reta commented on a change in pull request #407: CXF-7716 Reduce StringBuilders and other performance changes. URL: https://github.com/apache/cxf/pull/407#discussion_r186569414 ########## File path: rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/JAXRSUtils.java ########## @@ -1508,11 +1509,40 @@ public static boolean matchMimeTypes(MediaType requestContentType, } } } - return new ArrayList<>(supportedMimeTypeList); + } + + public static boolean doMimeTypesIntersect(List<MediaType> requiredMediaTypes, List<MediaType> userMediaTypes) { + for (MediaType requiredType : requiredMediaTypes) { Review comment: :+1: ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: users@infra.apache.org
            githubbot ASF GitHub Bot added a comment -

            WhiteCat22 commented on issue #407: CXF-7716 Reduce StringBuilders and other performance changes.
            URL: https://github.com/apache/cxf/pull/407#issuecomment-387525374

            @reta I've pushed the JAXRSUtils changes that we talked about.

            ----------------------------------------------------------------
            This is an automated message from the Apache Git Service.
            To respond to the message, please log on GitHub and use the
            URL above to go to the specific comment.

            For queries about this service, please contact Infrastructure at:
            users@infra.apache.org

            githubbot ASF GitHub Bot added a comment - WhiteCat22 commented on issue #407: CXF-7716 Reduce StringBuilders and other performance changes. URL: https://github.com/apache/cxf/pull/407#issuecomment-387525374 @reta I've pushed the JAXRSUtils changes that we talked about. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: users@infra.apache.org
            githubbot ASF GitHub Bot added a comment -

            reta commented on issue #407: CXF-7716 Reduce StringBuilders and other performance changes.
            URL: https://github.com/apache/cxf/pull/407#issuecomment-387586865

            @WhiteCat22 Looks good to me, thanks a lot. @asoldano any objections towards merging it? It is good set of micro-optimizations overall (in my opinion).

            ----------------------------------------------------------------
            This is an automated message from the Apache Git Service.
            To respond to the message, please log on GitHub and use the
            URL above to go to the specific comment.

            For queries about this service, please contact Infrastructure at:
            users@infra.apache.org

            githubbot ASF GitHub Bot added a comment - reta commented on issue #407: CXF-7716 Reduce StringBuilders and other performance changes. URL: https://github.com/apache/cxf/pull/407#issuecomment-387586865 @WhiteCat22 Looks good to me, thanks a lot. @asoldano any objections towards merging it? It is good set of micro-optimizations overall (in my opinion). ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: users@infra.apache.org
            githubbot ASF GitHub Bot added a comment -

            WhiteCat22 commented on issue #407: CXF-7716 Reduce StringBuilders and other performance changes.
            URL: https://github.com/apache/cxf/pull/407#issuecomment-387788090

            Can we also deliver this to 3.1.X? I'm not sure if that would require additional work to merge or not.

            ----------------------------------------------------------------
            This is an automated message from the Apache Git Service.
            To respond to the message, please log on GitHub and use the
            URL above to go to the specific comment.

            For queries about this service, please contact Infrastructure at:
            users@infra.apache.org

            githubbot ASF GitHub Bot added a comment - WhiteCat22 commented on issue #407: CXF-7716 Reduce StringBuilders and other performance changes. URL: https://github.com/apache/cxf/pull/407#issuecomment-387788090 Can we also deliver this to 3.1.X? I'm not sure if that would require additional work to merge or not. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: users@infra.apache.org
            githubbot ASF GitHub Bot added a comment -

            dkulp commented on issue #407: CXF-7716 Reduce StringBuilders and other performance changes.
            URL: https://github.com/apache/cxf/pull/407#issuecomment-388042074

            lgtm

            ----------------------------------------------------------------
            This is an automated message from the Apache Git Service.
            To respond to the message, please log on GitHub and use the
            URL above to go to the specific comment.

            For queries about this service, please contact Infrastructure at:
            users@infra.apache.org

            githubbot ASF GitHub Bot added a comment - dkulp commented on issue #407: CXF-7716 Reduce StringBuilders and other performance changes. URL: https://github.com/apache/cxf/pull/407#issuecomment-388042074 lgtm ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: users@infra.apache.org
            githubbot ASF GitHub Bot added a comment -

            reta commented on issue #407: CXF-7716 Reduce StringBuilders and other performance changes.
            URL: https://github.com/apache/cxf/pull/407#issuecomment-388198964

            Thanks guys! @WhiteCat22 I will take care of back-porting to 3.1.x

            ----------------------------------------------------------------
            This is an automated message from the Apache Git Service.
            To respond to the message, please log on GitHub and use the
            URL above to go to the specific comment.

            For queries about this service, please contact Infrastructure at:
            users@infra.apache.org

            githubbot ASF GitHub Bot added a comment - reta commented on issue #407: CXF-7716 Reduce StringBuilders and other performance changes. URL: https://github.com/apache/cxf/pull/407#issuecomment-388198964 Thanks guys! @WhiteCat22 I will take care of back-porting to 3.1.x ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: users@infra.apache.org
            githubbot ASF GitHub Bot added a comment -

            reta closed pull request #407: CXF-7716 Reduce StringBuilders and other performance changes.
            URL: https://github.com/apache/cxf/pull/407

            This is a PR merged from a forked repository.
            As GitHub hides the original diff on merge, it is displayed below for
            the sake of provenance:

            As this is a foreign pull request (from a fork), the diff is supplied
            below (as it won't show otherwise due to GitHub magic):

            diff --git a/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/lifecycle/PerRequestResourceProvider.java b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/lifecycle/PerRequestResourceProvider.java
            index 1bc98bd9def..61881fbbf8d 100644
            — a/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/lifecycle/PerRequestResourceProvider.java
            +++ b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/lifecycle/PerRequestResourceProvider.java
            @@ -19,9 +19,11 @@

            package org.apache.cxf.jaxrs.lifecycle;

            +import java.lang.annotation.Annotation;
            import java.lang.reflect.Constructor;
            import java.lang.reflect.InvocationTargetException;
            import java.lang.reflect.Method;
            +import java.lang.reflect.Type;
            import java.util.Collections;
            import java.util.Map;

            @@ -45,6 +47,9 @@
            private Constructor<?> c;
            private Method postConstructMethod;
            private Method preDestroyMethod;
            + private final Class<?>[] params;
            + private final Annotation[][] anns;
            + private final Type[] genericTypes;

            public PerRequestResourceProvider(Class<?> clazz) {
            c = ResourceUtils.findResourceConstructor(clazz, true);
            @@ -52,6 +57,9 @@ public PerRequestResourceProvider(Class<?> clazz)

            { throw new RuntimeException("Resource class " + clazz + " has no valid constructor"); }

            + params = c.getParameterTypes();
            + anns = c.getParameterAnnotations();
            + genericTypes = c.getGenericParameterTypes();
            postConstructMethod = ResourceUtils.findPostConstructMethod(clazz);
            preDestroyMethod = ResourceUtils.findPreDestroyMethod(clazz);
            }
            @@ -75,7 +83,7 @@ protected Object createInstance(Message m) {
            (ProviderInfo<?>)m.getExchange().getEndpoint().get(Application.class.getName());
            Map<Class<?>, Object> mapValues = CastUtils.cast(application == null ? null
            : Collections.singletonMap(Application.class, application.getProvider()));

            • Object[] values = ResourceUtils.createConstructorArguments(c, m, true, mapValues);
              + Object[] values = ResourceUtils.createConstructorArguments(c, m, true, mapValues, params, anns, genericTypes);
              try {
              Object instance = values.length > 0 ? c.newInstance(values) : c.newInstance(new Object[]{});
              InjectionUtils.invokeLifeCycleMethod(instance, postConstructMethod);
              diff --git a/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/model/URITemplate.java b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/model/URITemplate.java
              index 29777a8981b..b5059f3ddca 100644
                • a/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/model/URITemplate.java
                  +++ b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/model/URITemplate.java
                  @@ -130,10 +130,34 @@ public String getPatternValue() {

            private static String escapeCharacters(String expression) {

            • StringBuilder sb = new StringBuilder();
            • for (int i = 0; i < expression.length(); i++) {
            • char ch = expression.charAt;
            • sb.append(isReservedCharacter(ch) ? "
              " + ch : ch);
              + int length = expression.length();
              + int i = 0;
              + char ch = ' ';
              + for (; i < length; ++i)
              Unknown macro: {+ ch = expression.charAt(i);+ if (isReservedCharacter(ch)) { + break; + }+ }

              +
              + if (i == length)

              { + return expression; + }

              +
              + // Allows for up to 8 escaped characters before we start creating more
              + // StringBuilders. 8 is an arbitrary limit, but it seems to be
              + // sufficient in most cases.
              + StringBuilder sb = new StringBuilder(length + 8);
              + sb.append(expression, 0, i);
              + sb.append('
              ');
              + sb.append(ch);
              + ++i;
              + for (; i < length; ++i)

              Unknown macro: {+ ch = expression.charAt(i);+ if (isReservedCharacter(ch)) { + sb.append('\\'); + }+ sb.append(ch); }

              return sb.toString();
              }
              diff --git a/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/provider/ProviderFactory.java b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/provider/ProviderFactory.java
              index e62d3396e1c..03b6a5e64b8 100644

                • a/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/provider/ProviderFactory.java
                  +++ b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/provider/ProviderFactory.java
                  @@ -222,7 +222,7 @@ protected static Object createProvider(String className, Bus bus) {
                  if (argCls != null && argCls.isAssignableFrom(contextCls)) {
                  List<MediaType> mTypes = JAXRSUtils.getProduceTypes(
                  cr.getProvider().getClass().getAnnotation(Produces.class));
            • if (JAXRSUtils.intersectMimeTypes(mTypes, type).size() > 0) {
              + if (JAXRSUtils.doMimeTypesIntersect(mTypes, type)) { injectContextValues(cr, m); candidates.add((ContextResolver<T>)cr.getProvider()); }

              @@ -728,10 +728,7 @@ private void sortContextResolvers()

              { MessageBodyReader<?> ep = pi.getProvider(); List<MediaType> supportedMediaTypes = JAXRSUtils.getProviderConsumeTypes(ep); - List<MediaType> availableMimeTypes = - JAXRSUtils.intersectMimeTypes(Collections.singletonList(mediaType), supportedMediaTypes, false); - - return availableMimeTypes.size() != 0; + return JAXRSUtils.doMimeTypesIntersect(Collections.singletonList(mediaType), supportedMediaTypes); }

            private boolean isReadable(ProviderInfo<MessageBodyReader<?>> pi,
            @@ -752,11 +749,7 @@ private boolean isReadable(ProviderInfo<MessageBodyReader<?>> pi,
            MessageBodyWriter<?> ep = pi.getProvider();
            List<MediaType> supportedMediaTypes = JAXRSUtils.getProviderProduceTypes(ep);

            • List<MediaType> availableMimeTypes =
            • JAXRSUtils.intersectMimeTypes(Collections.singletonList(mediaType),
            • supportedMediaTypes, false);
              -
            • return availableMimeTypes.size() != 0;
              + return JAXRSUtils.doMimeTypesIntersect(Collections.singletonList(mediaType), supportedMediaTypes);
              }

            private boolean isWriteable(ProviderInfo<MessageBodyWriter<?>> pi,
            diff --git a/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/AccumulatingIntersector.java b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/AccumulatingIntersector.java
            new file mode 100644
            index 00000000000..3d2537f46c6
            — /dev/null
            +++ b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/AccumulatingIntersector.java
            @@ -0,0 +1,74 @@
            +/**
            + * Licensed to the Apache Software Foundation (ASF) under one
            + * or more contributor license agreements. See the NOTICE file
            + * distributed with this work for additional information
            + * regarding copyright ownership. The ASF licenses this file
            + * to you under the Apache License, Version 2.0 (the
            + * "License"); you may not use this file except in compliance
            + * with the License. You may obtain a copy of the License at
            + *
            + * http://www.apache.org/licenses/LICENSE-2.0
            + *
            + * Unless required by applicable law or agreed to in writing,
            + * software distributed under the License is distributed on an
            + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
            + * KIND, either express or implied. See the License for the
            + * specific language governing permissions and limitations
            + * under the License.
            + */
            +
            +package org.apache.cxf.jaxrs.utils;
            +
            +import java.util.LinkedHashMap;
            +import java.util.LinkedHashSet;
            +import java.util.Map;
            +import java.util.Set;
            +
            +import javax.ws.rs.core.MediaType;
            +
            +public class AccumulatingIntersector implements MimeTypesIntersector {
            + private static final String MEDIA_TYPE_DISTANCE_PARAM = "d";
            + private final Set<MediaType> supportedMimeTypeList = new LinkedHashSet<MediaType>();
            + private final boolean addRequiredParamsIfPossible;
            + private final boolean addDistanceParameter;
            +
            + AccumulatingIntersector(boolean addRequiredParamsIfPossible, boolean addDistanceParameter)

            { + this.addRequiredParamsIfPossible = addRequiredParamsIfPossible; + this.addDistanceParameter = addDistanceParameter; + }

            +
            + @Override
            + public boolean intersect(MediaType requiredType, MediaType userType) {
            + boolean requiredTypeWildcard = requiredType.getType().equals(MediaType.MEDIA_TYPE_WILDCARD);
            + boolean requiredSubTypeWildcard = requiredType.getSubtype().contains(MediaType.MEDIA_TYPE_WILDCARD);
            +
            + String type = requiredTypeWildcard ? userType.getType() : requiredType.getType();
            + String subtype = requiredSubTypeWildcard ? userType.getSubtype() : requiredType.getSubtype();
            +
            + Map<String, String> parameters = userType.getParameters();
            + if (addRequiredParamsIfPossible) {
            + parameters = new LinkedHashMap<String, String>(parameters);
            + for (Map.Entry<String, String> entry : requiredType.getParameters().entrySet()) {
            + if (!parameters.containsKey(entry.getKey()))

            { + parameters.put(entry.getKey(), entry.getValue()); + }

            + }
            + }
            + if (addDistanceParameter) {
            + int distance = 0;
            + if (requiredTypeWildcard)

            { + distance++; + }
            + if (requiredSubTypeWildcard) {+ distance++;+ }

            + parameters.put(MEDIA_TYPE_DISTANCE_PARAM, Integer.toString(distance));
            + }
            + getSupportedMimeTypeList().add(new MediaType(type, subtype, parameters));
            + return true;
            + }
            +
            + public Set<MediaType> getSupportedMimeTypeList()

            { + return supportedMimeTypeList; + }

            +}
            diff --git a/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/HttpUtils.java b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/HttpUtils.java
            index 41286c358fb..53a21434fe3 100644
            — a/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/HttpUtils.java
            +++ b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/HttpUtils.java
            @@ -117,24 +117,29 @@ public static String pathDecode(String value) {

            private static String componentEncode(String reservedChars, String value) {

            • StringBuilder buffer = new StringBuilder();
            • StringBuilder bufferToEncode = new StringBuilder();
              -
            • for (int i = 0; i < value.length(); i++) {
              + StringBuilder buffer = null;
              + int length = value.length();
              + int startingIndex = 0;
              + for (int i = 0; i < length; i++) {
              char currentChar = value.charAt;
              if (reservedChars.indexOf(currentChar) != -1) {
            • if (bufferToEncode.length() > 0) {
            • buffer.append(urlEncode(bufferToEncode.toString()));
            • bufferToEncode.setLength(0);
              + if (buffer == null) { + buffer = new StringBuilder(length + 8); + }

              + // If it is going to be an empty string nothing to encode.
              + if (i != startingIndex)

              { + buffer.append(urlEncode(value.substring(startingIndex, i))); }

              buffer.append(currentChar);

            • } else { - bufferToEncode.append(currentChar); + startingIndex = i + 1; }

              }

            • if (bufferToEncode.length() > 0) {
            • buffer.append(urlEncode(bufferToEncode.toString()));
              + if (buffer == null) { + return urlEncode(value); + }

              + if (startingIndex < length)

              { + buffer.append(urlEncode(value.substring(startingIndex, length))); }

            return buffer.toString();
            @@ -186,15 +191,21 @@ public static String encodePartiallyEncoded(String encoded, boolean query)

            { return encoded; }

            Matcher m = ENCODE_PATTERN.matcher(encoded);

            • StringBuilder sb = new StringBuilder();
              +
              + if (!m.find()) { + return query ? HttpUtils.queryEncode(encoded) : HttpUtils.pathEncode(encoded); + }

              +
              + int length = encoded.length();
              + StringBuilder sb = new StringBuilder(length + 8);
              int i = 0;

            • while (m.find()) {
              + do { String before = encoded.substring(i, m.start()); sb.append(query ? HttpUtils.queryEncode(before) : HttpUtils.pathEncode(before)); sb.append(m.group()); i = m.end(); - }
            • String tail = encoded.substring(i, encoded.length());
              + } while (m.find());
              + String tail = encoded.substring(i, length);
              sb.append(query ? HttpUtils.queryEncode(tail) : HttpUtils.pathEncode(tail));
              return sb.toString();
              }
              diff --git a/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/JAXRSUtils.java b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/JAXRSUtils.java
              index 3094a3eb767..252f4cd06b3 100644
                • a/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/JAXRSUtils.java
                  +++ b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/JAXRSUtils.java
                  @@ -35,7 +35,6 @@
                  import java.util.Comparator;
                  import java.util.HashSet;
                  import java.util.LinkedHashMap;
                  -import java.util.LinkedHashSet;
                  import java.util.LinkedList;
                  import java.util.List;
                  import java.util.Map;
                  @@ -1400,21 +1399,21 @@ public static void writeMessageBody(List<WriterInterceptor> writers,
                  public static boolean matchConsumeTypes(MediaType requestContentType,
                  OperationResourceInfo ori) { - return !intersectMimeTypes(ori.getConsumeTypes(), requestContentType).isEmpty(); + return doMimeTypesIntersect(ori.getConsumeTypes(), requestContentType); }

            public static boolean matchProduceTypes(MediaType acceptContentType,
            OperationResourceInfo ori)

            { - return !intersectMimeTypes(ori.getProduceTypes(), acceptContentType).isEmpty(); + return doMimeTypesIntersect(ori.getProduceTypes(), acceptContentType); }

            public static boolean matchMimeTypes(MediaType requestContentType,
            MediaType acceptContentType,
            OperationResourceInfo ori)

            { - return intersectMimeTypes(ori.getConsumeTypes(), requestContentType).size() != 0 - && intersectMimeTypes(ori.getProduceTypes(), acceptContentType).size() != 0; + return doMimeTypesIntersect(ori.getConsumeTypes(), requestContentType) + && doMimeTypesIntersect(ori.getProduceTypes(), acceptContentType); }

            public static List<MediaType> parseMediaTypes(String types)

            { @@ -1439,6 +1438,16 @@ public static boolean matchMimeTypes(MediaType requestContentType, return acceptValues; }

            + public static boolean doMimeTypesIntersect(List<MediaType> mimeTypesA, MediaType mimeTypeB)

            { + return doMimeTypesIntersect(mimeTypesA, Collections.singletonList(mimeTypeB)); + }

            +
            + public static boolean doMimeTypesIntersect(List<MediaType> requiredMediaTypes, List<MediaType> userMediaTypes)

            { + final NonAccumulatingIntersector intersector = new NonAccumulatingIntersector(); + intersectMimeTypes(requiredMediaTypes, userMediaTypes, intersector); + return intersector.doIntersect(); + }

            +
            /**

            • intersect two mime types
              *
              @@ -1451,11 +1460,19 @@ public static boolean matchMimeTypes(MediaType requestContentType,
              boolean addRequiredParamsIfPossible) { return intersectMimeTypes(requiredMediaTypes, userMediaTypes, addRequiredParamsIfPossible, false); }

              +
              public static List<MediaType> intersectMimeTypes(List<MediaType> requiredMediaTypes,
              List<MediaType> userMediaTypes,
              boolean addRequiredParamsIfPossible,
              boolean addDistanceParameter)

              { - Set<MediaType> supportedMimeTypeList = new LinkedHashSet<MediaType>(); + final AccumulatingIntersector intersector = new AccumulatingIntersector(addRequiredParamsIfPossible, + addDistanceParameter); + intersectMimeTypes(requiredMediaTypes, userMediaTypes, intersector); + return new ArrayList<>(intersector.getSupportedMimeTypeList()); + }

              +
              + private static void intersectMimeTypes(List<MediaType> requiredMediaTypes, List<MediaType> userMediaTypes,
              + MimeTypesIntersector intersector) {

            for (MediaType requiredType : requiredMediaTypes) {
            for (MediaType userType : userMediaTypes) {
            @@ -1464,12 +1481,10 @@ public static boolean matchMimeTypes(MediaType requestContentType,
            boolean parametersMatched = true;
            for (Map.Entry<String, String> entry : userType.getParameters().entrySet()) {
            String value = requiredType.getParameters().get(entry.getKey());

            • if (value != null && entry.getValue() != null
            • && !(stripDoubleQuotesIfNeeded(value).equals(
            • stripDoubleQuotesIfNeeded(entry.getValue())))) {
            • if (HTTP_CHARSET_PARAM.equals(entry.getKey())
            • && value.equalsIgnoreCase(entry.getValue())) {
              + if (value != null && entry.getValue() != null && !(stripDoubleQuotesIfNeeded(value)
              + .equals(stripDoubleQuotesIfNeeded(entry.getValue())))) {
              +
              + if (HTTP_CHARSET_PARAM.equals(entry.getKey()) && value.equalsIgnoreCase(entry.getValue())) { continue; }

              parametersMatched = false;
              @@ -1479,40 +1494,15 @@ public static boolean matchMimeTypes(MediaType requestContentType,
              if (!parametersMatched)

              { continue; }
            • boolean requiredTypeWildcard = requiredType.getType().equals(MediaType.MEDIA_TYPE_WILDCARD);
            • boolean requiredSubTypeWildcard = requiredType.getSubtype().contains(MediaType.MEDIA_TYPE_WILDCARD);
              -
            • String type = requiredTypeWildcard ? userType.getType() : requiredType.getType();
            • String subtype = requiredSubTypeWildcard ? userType.getSubtype() : requiredType.getSubtype();
              -
            • Map<String, String> parameters = userType.getParameters();
            • if (addRequiredParamsIfPossible) {
            • parameters = new LinkedHashMap<String, String>(parameters);
            • for (Map.Entry<String, String> entry : requiredType.getParameters().entrySet()) {
            • if (!parameters.containsKey(entry.getKey())) { - parameters.put(entry.getKey(), entry.getValue()); - }
            • }
            • }
            • if (addDistanceParameter) {
            • int distance = 0;
            • if (requiredTypeWildcard) { - distance++; - }
              - if (requiredSubTypeWildcard) {- distance++;- }
            • parameters.put(MEDIA_TYPE_DISTANCE_PARAM, Integer.toString(distance));
              +
              + if (!intersector.intersect(requiredType, userType)) { + return; }
            • supportedMimeTypeList.add(new MediaType(type, subtype, parameters));
              }
              }
              }
              -
            • return new ArrayList<>(supportedMimeTypeList);
              -
              }
            • +
              private static String stripDoubleQuotesIfNeeded(String value) {
              if (value != null && value.startsWith("\"")
              && value.endsWith("\"") && value.length() > 1) {
              diff --git a/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/MimeTypesIntersector.java b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/MimeTypesIntersector.java
              new file mode 100644
              index 00000000000..3dd3e47be76

                • /dev/null
                  +++ b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/MimeTypesIntersector.java
                  @@ -0,0 +1,26 @@
                  +/**
                  + * Licensed to the Apache Software Foundation (ASF) under one
                  + * or more contributor license agreements. See the NOTICE file
                  + * distributed with this work for additional information
                  + * regarding copyright ownership. The ASF licenses this file
                  + * to you under the Apache License, Version 2.0 (the
                  + * "License"); you may not use this file except in compliance
                  + * with the License. You may obtain a copy of the License at
                  + *
                  + * http://www.apache.org/licenses/LICENSE-2.0
                  + *
                  + * Unless required by applicable law or agreed to in writing,
                  + * software distributed under the License is distributed on an
                  + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
                  + * KIND, either express or implied. See the License for the
                  + * specific language governing permissions and limitations
                  + * under the License.
                  + */
                  +
                  +package org.apache.cxf.jaxrs.utils;
                  +
                  +import javax.ws.rs.core.MediaType;
                  +
                  +public interface MimeTypesIntersector { + boolean intersect(MediaType requiredType, MediaType userType); +}

                  diff --git a/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/NonAccumulatingIntersector.java b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/NonAccumulatingIntersector.java
                  new file mode 100644
                  index 00000000000..7f6661e6c39

                • /dev/null
                  +++ b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/NonAccumulatingIntersector.java
                  @@ -0,0 +1,36 @@
                  +/**
                  + * Licensed to the Apache Software Foundation (ASF) under one
                  + * or more contributor license agreements. See the NOTICE file
                  + * distributed with this work for additional information
                  + * regarding copyright ownership. The ASF licenses this file
                  + * to you under the Apache License, Version 2.0 (the
                  + * "License"); you may not use this file except in compliance
                  + * with the License. You may obtain a copy of the License at
                  + *
                  + * http://www.apache.org/licenses/LICENSE-2.0
                  + *
                  + * Unless required by applicable law or agreed to in writing,
                  + * software distributed under the License is distributed on an
                  + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
                  + * KIND, either express or implied. See the License for the
                  + * specific language governing permissions and limitations
                  + * under the License.
                  + */
                  +
                  +package org.apache.cxf.jaxrs.utils;
                  +
                  +import javax.ws.rs.core.MediaType;
                  +
                  +public class NonAccumulatingIntersector implements MimeTypesIntersector
                  Unknown macro: {+ private boolean doIntersect;++ @Override+ public boolean intersect(MediaType requiredType, MediaType userType) { + doIntersect = true; + return false; + }++ public boolean doIntersect() { + return doIntersect; + }+}

                  diff --git a/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/ResourceUtils.java b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/ResourceUtils.java
                  index 39d97696a54..1b32e855f25 100644

                • a/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/ResourceUtils.java
                  +++ b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/ResourceUtils.java
                  @@ -816,12 +816,23 @@ private static UserOperation getOperationFromElement(Element e) {
                  Message m,
                  boolean perRequest,
                  Map<Class<?>, Object> contextValues) {
            • if (m == null) { - m = new MessageImpl(); - }

              Class<?>[] params = c.getParameterTypes();
              Annotation[][] anns = c.getParameterAnnotations();
              Type[] genericTypes = c.getGenericParameterTypes();
              + return createConstructorArguments(c, m, perRequest, contextValues, params, anns, genericTypes);
              + }
              +
              + public static Object[] createConstructorArguments(Constructor<?> c,
              + Message m,
              + boolean perRequest,
              + Map<Class<?>,
              + Object> contextValues,
              + Class<?>[] params,
              + Annotation[][] anns,
              + Type[] genericTypes) {
              + if (m == null)

              { + m = new MessageImpl(); + }

              @SuppressWarnings("unchecked")
              MultivaluedMap<String, String> templateValues =
              (MultivaluedMap<String, String>)m.get(URITemplate.TEMPLATE_PARAMETERS);

            ----------------------------------------------------------------
            This is an automated message from the Apache Git Service.
            To respond to the message, please log on GitHub and use the
            URL above to go to the specific comment.

            For queries about this service, please contact Infrastructure at:
            users@infra.apache.org

            githubbot ASF GitHub Bot added a comment - reta closed pull request #407: CXF-7716 Reduce StringBuilders and other performance changes. URL: https://github.com/apache/cxf/pull/407 This is a PR merged from a forked repository. As GitHub hides the original diff on merge, it is displayed below for the sake of provenance: As this is a foreign pull request (from a fork), the diff is supplied below (as it won't show otherwise due to GitHub magic): diff --git a/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/lifecycle/PerRequestResourceProvider.java b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/lifecycle/PerRequestResourceProvider.java index 1bc98bd9def..61881fbbf8d 100644 — a/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/lifecycle/PerRequestResourceProvider.java +++ b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/lifecycle/PerRequestResourceProvider.java @@ -19,9 +19,11 @@ package org.apache.cxf.jaxrs.lifecycle; +import java.lang.annotation.Annotation; import java.lang.reflect.Constructor; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; +import java.lang.reflect.Type; import java.util.Collections; import java.util.Map; @@ -45,6 +47,9 @@ private Constructor<?> c; private Method postConstructMethod; private Method preDestroyMethod; + private final Class<?>[] params; + private final Annotation[][] anns; + private final Type[] genericTypes; public PerRequestResourceProvider(Class<?> clazz) { c = ResourceUtils.findResourceConstructor(clazz, true); @@ -52,6 +57,9 @@ public PerRequestResourceProvider(Class<?> clazz) { throw new RuntimeException("Resource class " + clazz + " has no valid constructor"); } + params = c.getParameterTypes(); + anns = c.getParameterAnnotations(); + genericTypes = c.getGenericParameterTypes(); postConstructMethod = ResourceUtils.findPostConstructMethod(clazz); preDestroyMethod = ResourceUtils.findPreDestroyMethod(clazz); } @@ -75,7 +83,7 @@ protected Object createInstance(Message m) { (ProviderInfo<?>)m.getExchange().getEndpoint().get(Application.class.getName()); Map<Class<?>, Object> mapValues = CastUtils.cast(application == null ? null : Collections.singletonMap(Application.class, application.getProvider())); Object[] values = ResourceUtils.createConstructorArguments(c, m, true, mapValues); + Object[] values = ResourceUtils.createConstructorArguments(c, m, true, mapValues, params, anns, genericTypes); try { Object instance = values.length > 0 ? c.newInstance(values) : c.newInstance(new Object[]{}); InjectionUtils.invokeLifeCycleMethod(instance, postConstructMethod); diff --git a/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/model/URITemplate.java b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/model/URITemplate.java index 29777a8981b..b5059f3ddca 100644 a/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/model/URITemplate.java +++ b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/model/URITemplate.java @@ -130,10 +130,34 @@ public String getPatternValue() { private static String escapeCharacters(String expression) { StringBuilder sb = new StringBuilder(); for (int i = 0; i < expression.length(); i++) { char ch = expression.charAt ; sb.append(isReservedCharacter(ch) ? " " + ch : ch); + int length = expression.length(); + int i = 0; + char ch = ' '; + for (; i < length; ++i) Unknown macro: {+ ch = expression.charAt(i);+ if (isReservedCharacter(ch)) { + break; + }+ } + + if (i == length) { + return expression; + } + + // Allows for up to 8 escaped characters before we start creating more + // StringBuilders. 8 is an arbitrary limit, but it seems to be + // sufficient in most cases. + StringBuilder sb = new StringBuilder(length + 8); + sb.append(expression, 0, i); + sb.append(' '); + sb.append(ch); + ++i; + for (; i < length; ++i) Unknown macro: {+ ch = expression.charAt(i);+ if (isReservedCharacter(ch)) { + sb.append('\\'); + }+ sb.append(ch); } return sb.toString(); } diff --git a/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/provider/ProviderFactory.java b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/provider/ProviderFactory.java index e62d3396e1c..03b6a5e64b8 100644 a/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/provider/ProviderFactory.java +++ b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/provider/ProviderFactory.java @@ -222,7 +222,7 @@ protected static Object createProvider(String className, Bus bus) { if (argCls != null && argCls.isAssignableFrom(contextCls)) { List<MediaType> mTypes = JAXRSUtils.getProduceTypes( cr.getProvider().getClass().getAnnotation(Produces.class)); if (JAXRSUtils.intersectMimeTypes(mTypes, type).size() > 0) { + if (JAXRSUtils.doMimeTypesIntersect(mTypes, type)) { injectContextValues(cr, m); candidates.add((ContextResolver<T>)cr.getProvider()); } @@ -728,10 +728,7 @@ private void sortContextResolvers() { MessageBodyReader<?> ep = pi.getProvider(); List<MediaType> supportedMediaTypes = JAXRSUtils.getProviderConsumeTypes(ep); - List<MediaType> availableMimeTypes = - JAXRSUtils.intersectMimeTypes(Collections.singletonList(mediaType), supportedMediaTypes, false); - - return availableMimeTypes.size() != 0; + return JAXRSUtils.doMimeTypesIntersect(Collections.singletonList(mediaType), supportedMediaTypes); } private boolean isReadable(ProviderInfo<MessageBodyReader<?>> pi, @@ -752,11 +749,7 @@ private boolean isReadable(ProviderInfo<MessageBodyReader<?>> pi, MessageBodyWriter<?> ep = pi.getProvider(); List<MediaType> supportedMediaTypes = JAXRSUtils.getProviderProduceTypes(ep); List<MediaType> availableMimeTypes = JAXRSUtils.intersectMimeTypes(Collections.singletonList(mediaType), supportedMediaTypes, false); - return availableMimeTypes.size() != 0; + return JAXRSUtils.doMimeTypesIntersect(Collections.singletonList(mediaType), supportedMediaTypes); } private boolean isWriteable(ProviderInfo<MessageBodyWriter<?>> pi, diff --git a/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/AccumulatingIntersector.java b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/AccumulatingIntersector.java new file mode 100644 index 00000000000..3d2537f46c6 — /dev/null +++ b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/AccumulatingIntersector.java @@ -0,0 +1,74 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.cxf.jaxrs.utils; + +import java.util.LinkedHashMap; +import java.util.LinkedHashSet; +import java.util.Map; +import java.util.Set; + +import javax.ws.rs.core.MediaType; + +public class AccumulatingIntersector implements MimeTypesIntersector { + private static final String MEDIA_TYPE_DISTANCE_PARAM = "d"; + private final Set<MediaType> supportedMimeTypeList = new LinkedHashSet<MediaType>(); + private final boolean addRequiredParamsIfPossible; + private final boolean addDistanceParameter; + + AccumulatingIntersector(boolean addRequiredParamsIfPossible, boolean addDistanceParameter) { + this.addRequiredParamsIfPossible = addRequiredParamsIfPossible; + this.addDistanceParameter = addDistanceParameter; + } + + @Override + public boolean intersect(MediaType requiredType, MediaType userType) { + boolean requiredTypeWildcard = requiredType.getType().equals(MediaType.MEDIA_TYPE_WILDCARD); + boolean requiredSubTypeWildcard = requiredType.getSubtype().contains(MediaType.MEDIA_TYPE_WILDCARD); + + String type = requiredTypeWildcard ? userType.getType() : requiredType.getType(); + String subtype = requiredSubTypeWildcard ? userType.getSubtype() : requiredType.getSubtype(); + + Map<String, String> parameters = userType.getParameters(); + if (addRequiredParamsIfPossible) { + parameters = new LinkedHashMap<String, String>(parameters); + for (Map.Entry<String, String> entry : requiredType.getParameters().entrySet()) { + if (!parameters.containsKey(entry.getKey())) { + parameters.put(entry.getKey(), entry.getValue()); + } + } + } + if (addDistanceParameter) { + int distance = 0; + if (requiredTypeWildcard) { + distance++; + } + if (requiredSubTypeWildcard) {+ distance++;+ } + parameters.put(MEDIA_TYPE_DISTANCE_PARAM, Integer.toString(distance)); + } + getSupportedMimeTypeList().add(new MediaType(type, subtype, parameters)); + return true; + } + + public Set<MediaType> getSupportedMimeTypeList() { + return supportedMimeTypeList; + } +} diff --git a/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/HttpUtils.java b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/HttpUtils.java index 41286c358fb..53a21434fe3 100644 — a/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/HttpUtils.java +++ b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/HttpUtils.java @@ -117,24 +117,29 @@ public static String pathDecode(String value) { private static String componentEncode(String reservedChars, String value) { StringBuilder buffer = new StringBuilder(); StringBuilder bufferToEncode = new StringBuilder(); - for (int i = 0; i < value.length(); i++) { + StringBuilder buffer = null; + int length = value.length(); + int startingIndex = 0; + for (int i = 0; i < length; i++) { char currentChar = value.charAt ; if (reservedChars.indexOf(currentChar) != -1) { if (bufferToEncode.length() > 0) { buffer.append(urlEncode(bufferToEncode.toString())); bufferToEncode.setLength(0); + if (buffer == null) { + buffer = new StringBuilder(length + 8); + } + // If it is going to be an empty string nothing to encode. + if (i != startingIndex) { + buffer.append(urlEncode(value.substring(startingIndex, i))); } buffer.append(currentChar); } else { - bufferToEncode.append(currentChar); + startingIndex = i + 1; } } if (bufferToEncode.length() > 0) { buffer.append(urlEncode(bufferToEncode.toString())); + if (buffer == null) { + return urlEncode(value); + } + if (startingIndex < length) { + buffer.append(urlEncode(value.substring(startingIndex, length))); } return buffer.toString(); @@ -186,15 +191,21 @@ public static String encodePartiallyEncoded(String encoded, boolean query) { return encoded; } Matcher m = ENCODE_PATTERN.matcher(encoded); StringBuilder sb = new StringBuilder(); + + if (!m.find()) { + return query ? HttpUtils.queryEncode(encoded) : HttpUtils.pathEncode(encoded); + } + + int length = encoded.length(); + StringBuilder sb = new StringBuilder(length + 8); int i = 0; while (m.find()) { + do { String before = encoded.substring(i, m.start()); sb.append(query ? HttpUtils.queryEncode(before) : HttpUtils.pathEncode(before)); sb.append(m.group()); i = m.end(); - } String tail = encoded.substring(i, encoded.length()); + } while (m.find()); + String tail = encoded.substring(i, length); sb.append(query ? HttpUtils.queryEncode(tail) : HttpUtils.pathEncode(tail)); return sb.toString(); } diff --git a/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/JAXRSUtils.java b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/JAXRSUtils.java index 3094a3eb767..252f4cd06b3 100644 a/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/JAXRSUtils.java +++ b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/JAXRSUtils.java @@ -35,7 +35,6 @@ import java.util.Comparator; import java.util.HashSet; import java.util.LinkedHashMap; -import java.util.LinkedHashSet; import java.util.LinkedList; import java.util.List; import java.util.Map; @@ -1400,21 +1399,21 @@ public static void writeMessageBody(List<WriterInterceptor> writers, public static boolean matchConsumeTypes(MediaType requestContentType, OperationResourceInfo ori) { - return !intersectMimeTypes(ori.getConsumeTypes(), requestContentType).isEmpty(); + return doMimeTypesIntersect(ori.getConsumeTypes(), requestContentType); } public static boolean matchProduceTypes(MediaType acceptContentType, OperationResourceInfo ori) { - return !intersectMimeTypes(ori.getProduceTypes(), acceptContentType).isEmpty(); + return doMimeTypesIntersect(ori.getProduceTypes(), acceptContentType); } public static boolean matchMimeTypes(MediaType requestContentType, MediaType acceptContentType, OperationResourceInfo ori) { - return intersectMimeTypes(ori.getConsumeTypes(), requestContentType).size() != 0 - && intersectMimeTypes(ori.getProduceTypes(), acceptContentType).size() != 0; + return doMimeTypesIntersect(ori.getConsumeTypes(), requestContentType) + && doMimeTypesIntersect(ori.getProduceTypes(), acceptContentType); } public static List<MediaType> parseMediaTypes(String types) { @@ -1439,6 +1438,16 @@ public static boolean matchMimeTypes(MediaType requestContentType, return acceptValues; } + public static boolean doMimeTypesIntersect(List<MediaType> mimeTypesA, MediaType mimeTypeB) { + return doMimeTypesIntersect(mimeTypesA, Collections.singletonList(mimeTypeB)); + } + + public static boolean doMimeTypesIntersect(List<MediaType> requiredMediaTypes, List<MediaType> userMediaTypes) { + final NonAccumulatingIntersector intersector = new NonAccumulatingIntersector(); + intersectMimeTypes(requiredMediaTypes, userMediaTypes, intersector); + return intersector.doIntersect(); + } + /** intersect two mime types * @@ -1451,11 +1460,19 @@ public static boolean matchMimeTypes(MediaType requestContentType, boolean addRequiredParamsIfPossible) { return intersectMimeTypes(requiredMediaTypes, userMediaTypes, addRequiredParamsIfPossible, false); } + public static List<MediaType> intersectMimeTypes(List<MediaType> requiredMediaTypes, List<MediaType> userMediaTypes, boolean addRequiredParamsIfPossible, boolean addDistanceParameter) { - Set<MediaType> supportedMimeTypeList = new LinkedHashSet<MediaType>(); + final AccumulatingIntersector intersector = new AccumulatingIntersector(addRequiredParamsIfPossible, + addDistanceParameter); + intersectMimeTypes(requiredMediaTypes, userMediaTypes, intersector); + return new ArrayList<>(intersector.getSupportedMimeTypeList()); + } + + private static void intersectMimeTypes(List<MediaType> requiredMediaTypes, List<MediaType> userMediaTypes, + MimeTypesIntersector intersector) { for (MediaType requiredType : requiredMediaTypes) { for (MediaType userType : userMediaTypes) { @@ -1464,12 +1481,10 @@ public static boolean matchMimeTypes(MediaType requestContentType, boolean parametersMatched = true; for (Map.Entry<String, String> entry : userType.getParameters().entrySet()) { String value = requiredType.getParameters().get(entry.getKey()); if (value != null && entry.getValue() != null && !(stripDoubleQuotesIfNeeded(value).equals( stripDoubleQuotesIfNeeded(entry.getValue())))) { if (HTTP_CHARSET_PARAM.equals(entry.getKey()) && value.equalsIgnoreCase(entry.getValue())) { + if (value != null && entry.getValue() != null && !(stripDoubleQuotesIfNeeded(value) + .equals(stripDoubleQuotesIfNeeded(entry.getValue())))) { + + if (HTTP_CHARSET_PARAM.equals(entry.getKey()) && value.equalsIgnoreCase(entry.getValue())) { continue; } parametersMatched = false; @@ -1479,40 +1494,15 @@ public static boolean matchMimeTypes(MediaType requestContentType, if (!parametersMatched) { continue; } boolean requiredTypeWildcard = requiredType.getType().equals(MediaType.MEDIA_TYPE_WILDCARD); boolean requiredSubTypeWildcard = requiredType.getSubtype().contains(MediaType.MEDIA_TYPE_WILDCARD); - String type = requiredTypeWildcard ? userType.getType() : requiredType.getType(); String subtype = requiredSubTypeWildcard ? userType.getSubtype() : requiredType.getSubtype(); - Map<String, String> parameters = userType.getParameters(); if (addRequiredParamsIfPossible) { parameters = new LinkedHashMap<String, String>(parameters); for (Map.Entry<String, String> entry : requiredType.getParameters().entrySet()) { if (!parameters.containsKey(entry.getKey())) { - parameters.put(entry.getKey(), entry.getValue()); - } } } if (addDistanceParameter) { int distance = 0; if (requiredTypeWildcard) { - distance++; - } - if (requiredSubTypeWildcard) {- distance++;- } parameters.put(MEDIA_TYPE_DISTANCE_PARAM, Integer.toString(distance)); + + if (!intersector.intersect(requiredType, userType)) { + return; } supportedMimeTypeList.add(new MediaType(type, subtype, parameters)); } } } - return new ArrayList<>(supportedMimeTypeList); - } + private static String stripDoubleQuotesIfNeeded(String value) { if (value != null && value.startsWith("\"") && value.endsWith("\"") && value.length() > 1) { diff --git a/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/MimeTypesIntersector.java b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/MimeTypesIntersector.java new file mode 100644 index 00000000000..3dd3e47be76 /dev/null +++ b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/MimeTypesIntersector.java @@ -0,0 +1,26 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.cxf.jaxrs.utils; + +import javax.ws.rs.core.MediaType; + +public interface MimeTypesIntersector { + boolean intersect(MediaType requiredType, MediaType userType); +} diff --git a/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/NonAccumulatingIntersector.java b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/NonAccumulatingIntersector.java new file mode 100644 index 00000000000..7f6661e6c39 /dev/null +++ b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/NonAccumulatingIntersector.java @@ -0,0 +1,36 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.cxf.jaxrs.utils; + +import javax.ws.rs.core.MediaType; + +public class NonAccumulatingIntersector implements MimeTypesIntersector Unknown macro: {+ private boolean doIntersect;++ @Override+ public boolean intersect(MediaType requiredType, MediaType userType) { + doIntersect = true; + return false; + }++ public boolean doIntersect() { + return doIntersect; + }+} diff --git a/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/ResourceUtils.java b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/ResourceUtils.java index 39d97696a54..1b32e855f25 100644 a/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/ResourceUtils.java +++ b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/ResourceUtils.java @@ -816,12 +816,23 @@ private static UserOperation getOperationFromElement(Element e) { Message m, boolean perRequest, Map<Class<?>, Object> contextValues) { if (m == null) { - m = new MessageImpl(); - } Class<?>[] params = c.getParameterTypes(); Annotation[][] anns = c.getParameterAnnotations(); Type[] genericTypes = c.getGenericParameterTypes(); + return createConstructorArguments(c, m, perRequest, contextValues, params, anns, genericTypes); + } + + public static Object[] createConstructorArguments(Constructor<?> c, + Message m, + boolean perRequest, + Map<Class<?>, + Object> contextValues, + Class<?>[] params, + Annotation[][] anns, + Type[] genericTypes) { + if (m == null) { + m = new MessageImpl(); + } @SuppressWarnings("unchecked") MultivaluedMap<String, String> templateValues = (MultivaluedMap<String, String>)m.get(URITemplate.TEMPLATE_PARAMETERS); ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: users@infra.apache.org
            githubbot ASF GitHub Bot added a comment -

            WhiteCat22 commented on issue #407: CXF-7716 Reduce StringBuilders and other performance changes.
            URL: https://github.com/apache/cxf/pull/407#issuecomment-388206050

            @reta Thanks!

            ----------------------------------------------------------------
            This is an automated message from the Apache Git Service.
            To respond to the message, please log on GitHub and use the
            URL above to go to the specific comment.

            For queries about this service, please contact Infrastructure at:
            users@infra.apache.org

            githubbot ASF GitHub Bot added a comment - WhiteCat22 commented on issue #407: CXF-7716 Reduce StringBuilders and other performance changes. URL: https://github.com/apache/cxf/pull/407#issuecomment-388206050 @reta Thanks! ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: users@infra.apache.org

            People

              Unassigned Unassigned
              atanders Adam Anderson
              Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Time Tracking

                  Estimated:
                  Original Estimate - 1h
                  1h
                  Remaining:
                  Remaining Estimate - 1h
                  1h
                  Logged:
                  Time Spent - Not Specified
                  Not Specified