[PR] MINOR: reduce allocations construction RecordHeaders [kafka]

["winfriedgerlach (via GitHub)" <gi...@apache.org>]

winfriedgerlach opened a new pull request, #22641:
URL: https://github.com/apache/kafka/pull/22641

   I propose a small optimization for the `RecordHeaders(Header[])` constructor. Since this code is used on the hot-path for `Consumer.poll()` (via `Fetcher` --> `FetchCollector` --> `CompletedFetch`), I think this code should not be wasteful.
   
   Have a look at the original code:
   ```java
       public RecordHeaders(Header[] headers) {
           this(headers == null ? null : Arrays.asList(headers)); // (1)
       }
   
       public RecordHeaders(Iterable<Header> headers) {
           //Use efficient copy constructor if possible, fallback to iteration otherwise
           if (headers == null) {
               this.headers = new ArrayList<>();
           } else if (headers instanceof RecordHeaders) {
               this.headers = new ArrayList<>(((RecordHeaders) headers).headers);
           } else {
               this.headers = new ArrayList<>(); // (2)
               for (Header header : headers) { // (3)
                   Objects.requireNonNull(header, "Header cannot be null.");
                   this.headers.add(header);
               }
           }
       }
   ```
   Note:
   * If `RecordHeaders(Headers[])` is called with a **zero-length** array, `Arrays.asList()` at (1) creates an `ArrayList` which is then never used, since the `ArrayList` created at (2) will be the result (and the for-each loop will never be entered).
   * If `RecordHeaders(Headers[])` is called with a **non-zero-length** array, the list created with `Arrays.asList()` at (1) is also just a useless wrapper.
   * The for-each-loop at (3) is also wasteful in the `Array` use-case, since it creates a new `Iterator` via `ArrayList.iterator()`, while a for-each-loop over an array does not construct an object (cf. [JLS 14.14.2](https://docs.oracle.com/javase/specs/jls/se25/html/jls-14.html#jls-14.14.2)).
   
   Thus I propose a simple change that avoids both object creations: the `ArrayList` that's merely used "as transport" to the other constructor and the `Iterator` that's not needed for iterating an `Array`.


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

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

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

Re: [PR] MINOR: reduce allocations constructing RecordHeaders [kafka]

["winfriedgerlach (via GitHub)" <gi...@apache.org>]

winfriedgerlach commented on PR #22641:
URL: https://github.com/apache/kafka/pull/22641#issuecomment-4776261906

   fixed unused import


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

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

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

Re: [PR] MINOR: reduce allocations constructing RecordHeaders [kafka]

["mimaison (via GitHub)" <gi...@apache.org>]

mimaison commented on PR #22641:
URL: https://github.com/apache/kafka/pull/22641#issuecomment-4797887917

   Thanks for the PR. Can you rebase your branch onto trunk? That should resolve the CI failure.


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

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

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

Re: [PR] MINOR: reduce allocations constructing RecordHeaders [kafka]

["winfriedgerlach (via GitHub)" <gi...@apache.org>]

winfriedgerlach commented on PR #22641:
URL: https://github.com/apache/kafka/pull/22641#issuecomment-4802851529

   @mimaison rebases as requested


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

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

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

Re: [PR] MINOR: reduce allocations constructing RecordHeaders [kafka]

["mimaison (via GitHub)" <gi...@apache.org>]

mimaison merged PR #22641:
URL: https://github.com/apache/kafka/pull/22641


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

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

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