You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@wicket.apache.org by Martijn Dashorst <ma...@gmail.com> on 2016/11/12 16:56:58 UTC
Make MarkupContainer streamable?
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
Re: Make MarkupContainer streamable?
Posted by Martin Grigorov <mg...@apache.org>.
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 <
martijn.dashorst@gmail.com> 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 <an...@gmail.com>
> 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
>
Re: Make MarkupContainer streamable?
Posted by Martijn Dashorst <ma...@gmail.com>.
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 <an...@gmail.com> 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/73ac8c7e6da3ff9dd0244ad66c63944ba3d64c78
>
>
>
> 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
Re: Make MarkupContainer streamable?
Posted by Andrea Del Bene <an...@gmail.com>.
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/73ac8c7e6da3ff9dd0244ad66c63944ba3d64c78
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