> On May 24, 2015, 7:16 p.m., Ewen Cheslack-Postava wrote: > > clients/src/main/java/org/apache/kafka/common/network/Selector.java, line > > 226 > > <https://reviews.apache.org/r/34608/diff/1/?file=969830#file969830line226> > > > > One drawback to this is that we're now constantly reallocating these > > instead of just reusing the same ArrayLists repeatedly. > > > > I doubt this has a substantial impact, but given that this is core > > code, it' might be worth running some simple benchmark (e.g. > > ProducerPerformance) just to sanity check that we don't see a perf drop > > over a long run where GC issues might become an issue. > > Jason Gustafson wrote: > I'll check it out. We could also reuse the same PollResult object, but > that sort of defeats the point of the change. I had hoped that by making the > underlying objects safer for threaded usage, some of the complexity in > synchronizing the KafkaConsumer would go away, but perhaps it should start > one layer up, at NetworkClient? > > Ewen Cheslack-Postava wrote: > Actually I think this is exactly the right approach and it's likely the > allocations don't matter much. They should all be very short lived so they > should be cheap -- they'll never make it out of the young generation anyway. > Still, it's a good idea to verify that assumption holds in practice.
Jay brought this point up in the JIRA ticket as well. I've been running the ProducerPerformance benchmark against trunk and haven't noticed a difference in GC behavior. Additionally, I added an early return from the poll method which returns an empty static PollResult when there is nothing to do. - Jason ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34608/#review85101 ----------------------------------------------------------- On May 26, 2015, 7:58 p.m., Jason Gustafson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/34608/ > ----------------------------------------------------------- > > (Updated May 26, 2015, 7:58 p.m.) > > > Review request for kafka. > > > Bugs: KAFKA-2217 > https://issues.apache.org/jira/browse/KAFKA-2217 > > > Repository: kafka > > > Description > ------- > > KAFKA-2217; updated for review comments > > > KAFKA-2217; add shortcut from poll when there's nothing to do > > > KAFKA-2217; update javadoc for new usage > > > KAFKA-2217; updated for review comments > > > Diffs > ----- > > clients/src/main/java/org/apache/kafka/clients/NetworkClient.java > 435fbb5116e80302eba11ed1d3069cb577dbdcbd > clients/src/main/java/org/apache/kafka/common/network/PollResult.java > PRE-CREATION > clients/src/main/java/org/apache/kafka/common/network/Selectable.java > b5f8d83e89f9026dc0853e5f92c00b2d7f043e22 > clients/src/main/java/org/apache/kafka/common/network/Selector.java > 57de0585e5e9a53eb9dcd99cac1ab3eb2086a302 > clients/src/test/java/org/apache/kafka/common/network/SelectorTest.java > d5b306b026e788b4e5479f3419805aa49ae889f3 > clients/src/test/java/org/apache/kafka/test/MockSelector.java > ea89b06a4c9e5bb351201299cd3037f5226f0e6c > > Diff: https://reviews.apache.org/r/34608/diff/ > > > Testing > ------- > > > Thanks, > > Jason Gustafson > >
