Thanks for the benchmarks [1] ! If the difference is that small then I'm fine to add these methods.
1. https://github.com/dashorst/wicket-benchmarks/blob/c6bd03c74887c5c777aebbe873598d99dfbc0833/src/main/java/com/martijndashorst/wicketbenchmarks/StreamVsGet.java Martin Grigorov Wicket Training and Consulting https://twitter.com/mtgrigorov On Sat, Nov 12, 2016 at 8:46 PM, Martijn Dashorst < [email protected]> wrote: > The performance hit isn't too big compared to iterating through the > components. Either with a stream() or a iterator() you're into O(n) > land. Then the O(n + c) where c is some time for the extra method > calls necessary for the stream() becomes negligible. This is > illustrated with the micro benchmark I just ran: > > Benchmark Mode Cnt Score > Error Units > retrieveComponentUsingGet thrpt 10 24964809.356 ± 195755.618 > ops/s > retrieveComponentUsingIterator thrpt 10 1274221.027 ± 31154.531 > ops/s > retrieveComponentUsingStream thrpt 10 1111762.631 ± 8065.395 > ops/s > > This is with 100 components added to a page, and retrieving component > with id "50" (each component was issued the index as a string as an > id). > > As you can see: using iterator or stream doesn't matter that much. > Using the get() is quote optimal, as we use a Map and index based > lookups when n > 30 or so. Therefore it is an order of magnitute > better: it performs at O(1). > > Here's the same benchmark, but now with 100,000 components added to the > page. > > Benchmark Mode Cnt Score > Error Units > StreamVsGet.retrieveComponentUsingGet thrpt 10 25653965.493 ± > 268342.736 ops/s > StreamVsGet.retrieveComponentUsingIterator thrpt 10 795.780 ± > 27.099 ops/s > StreamVsGet.retrieveComponentUsingStream thrpt 10 756.819 ± > 30.469 ops/s > > So in my opinion the performance loss is not too bad, at least when > you don't have a component identifier at your disposal. > > I would also add two stream() methods: > > - one to stream() the direct children of the markupcontainer > - one to hierarchically stream() all children > > The second one is competing with visitChildren(), so might be unnecessary. > > Martijn > > > On Sat, Nov 12, 2016 at 7:36 PM, Andrea Del Bene <[email protected]> > wrote: > > I did something similar some time ago. The idea is nice but it was > abandoned > > due to performance drawbacks as streams are quite slower than regular > > iteration. Anyway, this is what I did > > > > https://github.com/apache/wicket/commit/73ac8c7e6da3ff9dd0244ad66c6394 > 4ba3d64c78 > > > > > > > > On 12/11/2016 17:56, Martijn Dashorst wrote: > >> > >> In our code I often see code that (ugh) retrieves a component by id, > >> something like: > >> > >> public class SomePanel extends Panel { > >> ... > >> target.add(getPage().get("feedback")); > >> ... > >> } > >> > >> If we made MarkupContainer streamable, we could, in pseudo code, do > >> something like: > >> > >> getPage().stream().filter(c->c.getId().equals("feedback")).findFirst(); > >> > >> Which is not better than getPage().get("feedback") > >> > >> But it is more flexible checking for types, etc. > >> > >> The implementation is not that complicated: > >> > >> > >> /** > >> * Returns a sequential {@code Stream} with the children of this > >> markup container > >> * as its source. > >> * > >> * @return a sequential {@code Stream} over the children of this > >> markup container > >> * @since 8.0 > >> */ > >> public Stream<Component> stream() > >> { > >> return StreamSupport.stream(spliterator(), false); > >> } > >> > >> Good idea? > >> > >> Martijn > > > > > > > > -- > Become a Wicket expert, learn from the best: http://wicketinaction.com >
