> 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. > > Jason Gustafson wrote: > 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.
I profiled this using the ProducerPerformance benchmark and I don't see any difference in the heap behavior compared to trunk. If it ever becomes a problem, we could always do something like Netty's RecyclableArrayList without changing the interface. For now, it doesn't seem like an issue. - Jason ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34608/#review85101 ----------------------------------------------------------- On May 27, 2015, 10:52 p.m., Jason Gustafson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/34608/ > ----------------------------------------------------------- > > (Updated May 27, 2015, 10:52 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 > > > KAFKA-2217; rework shortcut logic to always record IO time > > > 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 > >
