You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@wicket.apache.org by Miguel Munoz <mm...@MainPointSystems.com> on 2008/04/04 21:03:33 UTC

PropertyResolver redesign

Comrades,

  After looking at the code for PropertyResolver, I find myself wondering if
there's a more efficent way to design it. It seems to me I could have more
efficient code if I could instantiate a resolver object which extracts the
getter and setter Methods on construction. Here's the situation I'm looking
at:

When displaying my data in a Repeating view, my sort code calls a Comparator
which looks something like this:

  public int compare(Object o1, Object o2) {
    String property = ... ;
    Object val1 = PropertyResolver.getValue( property, o1 );
    Object val2 = PropertyResolver.getValue( property, o2 );

    return valueComparator.compare( val1, val2 );
  }

For a given row, the sorter will call the getValue() method repeatedly, so I
want it to run quickly, but I can already see a problem:

The getValue() method needs to first extract a method from the property
name, then call that method to retrieve a value. That second step will
produce a different result for each row, but the first step, which extracts
the Method object, doesn't change from row to row, so it would be much more
efficent to do that part ahead of time. It seems to me that it shouldn't be
hard to write a new property resolver that I can instantiate, so the
extraction of the Method can be done ahead of time. In other words, I should
be able to write something like this:

  PropertyResolver2 resolver = new PropertyResolver2("propertyName",
MyBean.class);

At this point, the resolver object will have a Method instance, or, for a
series of properties, an array of Methods. Then my comparator would look
like this:

  public int compare(Object o1, Object o2) {  // o1 & o2 must be of class
MyBean.
    String property = ... ;
    Object val1 = resolver.getValue( o1 );
    Object val2 = resolver.getValue( o2 );

    return valueComparator.compare( val1, val2 );
  }

The PropertyResolver2 constructor would already have the Method extracted,
so the comparator would execute much faster.

So here's my question: Is there any reason why the Wicket code doesn't do it
this way? 

-- 
View this message in context: http://www.nabble.com/PropertyResolver-redesign-tp16495644p16495644.html
Sent from the Wicket - Dev mailing list archive at Nabble.com.


Re: PropertyResolver redesign

Posted by Matej Knopp <ma...@gmail.com>.
On Mon, Apr 7, 2008 at 10:01 PM, Matej Knopp <ma...@gmail.com> wrote:
> >  > Hi,
>  >  >
>  >  > first of all, are you sure property resolver is your bottleneck? I
>  >  > really have hard time believing that.
>  >  >
>  >  > PropertyResolver already caches the method instances based on target
>  >  > object class.
>  >  >
>  >
>  >  Using the PropertyResolver increased my sort time from 172 ms to 875 ms,
>  >  which is roughly a factor of 5. Keep in mind that, when sorting, the
>  >  getValue() method will get called many times for each element, so it doesn't
>  >  take much to seriously slow things down. In other tests, I found that, for
>  >  example, caching a CollationKey based on the Locale also slowed things down
>  >  a lot, so I'm not surprised by this at all.
>  >
>  >
>  Well, 875ms really is a lot. But i still have no idea what exactly you
>  are doing.
>
> >
>  >  Matej Knopp-2 wrote:
>  >  >
>  >  >
>  >  > Also your approach wouldn't work for things like "property1.property2"
>  >  > where property 2 container type depends on result of property 1
>  >  > evaluation.
>  >  >
>  >
>  >  Unless the type of property1 changes with different instances of the
>  >  original bean, I don't see why my approach wouldn't work. Even if the type
>  >  does change, the reflection code would find the interface or abstract class
>  >  with the getProperty2() method, so it still shouldn't be a problem. (I will
>  >  write some code to test these assumptions.)
>  No. It's called polymorphism, we can not realy on the return type.
>
>  So if you have method Object getProperty() we have to examine the
>  return type so that we can navigate further the hierarchy,
Correction, we have to examine the type of returned object.

>
>  -Matej
>
>
>  >
>  >  -----
>  >  There are 10 kinds of people: Those who know binary and those who don't.
>  >  --
>  >  View this message in context: http://www.nabble.com/PropertyResolver-redesign-tp16495644p16539209.html
>  >
>  >
>  > Sent from the Wicket - Dev mailing list archive at Nabble.com.
>  >
>  >
>
>
>
>
>
> --
>  Resizable and reorderable grid components.
>  http://www.inmethod.com
>



-- 
Resizable and reorderable grid components.
http://www.inmethod.com

Re: PropertyResolver redesign

Posted by Matej Knopp <ma...@gmail.com>.
>  > Hi,
>  >
>  > first of all, are you sure property resolver is your bottleneck? I
>  > really have hard time believing that.
>  >
>  > PropertyResolver already caches the method instances based on target
>  > object class.
>  >
>
>  Using the PropertyResolver increased my sort time from 172 ms to 875 ms,
>  which is roughly a factor of 5. Keep in mind that, when sorting, the
>  getValue() method will get called many times for each element, so it doesn't
>  take much to seriously slow things down. In other tests, I found that, for
>  example, caching a CollationKey based on the Locale also slowed things down
>  a lot, so I'm not surprised by this at all.
>
>
Well, 875ms really is a lot. But i still have no idea what exactly you
are doing.
>
>  Matej Knopp-2 wrote:
>  >
>  >
>  > Also your approach wouldn't work for things like "property1.property2"
>  > where property 2 container type depends on result of property 1
>  > evaluation.
>  >
>
>  Unless the type of property1 changes with different instances of the
>  original bean, I don't see why my approach wouldn't work. Even if the type
>  does change, the reflection code would find the interface or abstract class
>  with the getProperty2() method, so it still shouldn't be a problem. (I will
>  write some code to test these assumptions.)
No. It's called polymorphism, we can not realy on the return type.

So if you have method Object getProperty() we have to examine the
return type so that we can navigate further the hierarchy,

-Matej

>
>  -----
>  There are 10 kinds of people: Those who know binary and those who don't.
>  --
>  View this message in context: http://www.nabble.com/PropertyResolver-redesign-tp16495644p16539209.html
>
>
> Sent from the Wicket - Dev mailing list archive at Nabble.com.
>
>



-- 
Resizable and reorderable grid components.
http://www.inmethod.com

Re: PropertyResolver redesign

Posted by Miguel Munoz <mm...@MainPointSystems.com>.

Matej Knopp-2 wrote:
> 
> Hi,
> 
> first of all, are you sure property resolver is your bottleneck? I
> really have hard time believing that.
> 
> PropertyResolver already caches the method instances based on target
> object class. 
> 

Using the PropertyResolver increased my sort time from 172 ms to 875 ms,
which is roughly a factor of 5. Keep in mind that, when sorting, the
getValue() method will get called many times for each element, so it doesn't
take much to seriously slow things down. In other tests, I found that, for
example, caching a CollationKey based on the Locale also slowed things down
a lot, so I'm not surprised by this at all. 


Matej Knopp-2 wrote:
> 
> 
> Also your approach wouldn't work for things like "property1.property2"
> where property 2 container type depends on result of property 1
> evaluation.
> 

Unless the type of property1 changes with different instances of the
original bean, I don't see why my approach wouldn't work. Even if the type
does change, the reflection code would find the interface or abstract class
with the getProperty2() method, so it still shouldn't be a problem. (I will
write some code to test these assumptions.) 

-----
There are 10 kinds of people: Those who know binary and those who don't.
-- 
View this message in context: http://www.nabble.com/PropertyResolver-redesign-tp16495644p16539209.html
Sent from the Wicket - Dev mailing list archive at Nabble.com.


Re: PropertyResolver redesign

Posted by Matej Knopp <ma...@gmail.com>.
On Wed, Apr 9, 2008 at 1:15 AM, Miguel Munoz
<mm...@mainpointsystems.com> wrote:
>
>
>  Matej Knopp-2 wrote:
>  >
>  > Also your approach wouldn't work for things like "property1.property2"
>  > where property 2 container type depends on result of property 1
>  > evaluation.
>  >
>
>  I can see how it's easy to assume that this can't work, but I tried it, and
>  I had no trouble at all making it work with chained properties. It works
>  fine with both getters and setters. There is one caveat: It works only if
>  the method can be written using direct calls without any casting. So, for
>  the property chain "student.major.adviser", if you can write this:
>
>   Adviser adviser = thing.getStudent().getMajor().getAdviser();
>
>  then my approach works fine with property chaining. But if the statement
>  needs to look like this:
>
>   Adviser adviser =
>  ((ThesisProject)thing.getStudent().getMajor()).getAdviser();
>
>  then my approach won't work. (I'm not sure if PropertyResolver will work in
>  this case, but I suspect it will.)
And how is this different to what I've been saying? PropertyResolver
will work. It has to. That's why we are caching individual classes
only and not entire hierarchy.

-Matej
>
>
>
>  -----
>  There are 10 kinds of people: Those who know binary and those who don't.
>  --
>  View this message in context: http://www.nabble.com/PropertyResolver-redesign-tp16495644p16576062.html
>
>
> Sent from the Wicket - Dev mailing list archive at Nabble.com.
>
>



-- 
Resizable and reorderable grid components.
http://www.inmethod.com

Re: PropertyResolver redesign

Posted by Miguel Munoz <mm...@MainPointSystems.com>.

Matej Knopp-2 wrote:
> 
> Also your approach wouldn't work for things like "property1.property2"
> where property 2 container type depends on result of property 1
> evaluation.
> 

I can see how it's easy to assume that this can't work, but I tried it, and
I had no trouble at all making it work with chained properties. It works
fine with both getters and setters. There is one caveat: It works only if
the method can be written using direct calls without any casting. So, for
the property chain "student.major.adviser", if you can write this:

  Adviser adviser = thing.getStudent().getMajor().getAdviser();

then my approach works fine with property chaining. But if the statement
needs to look like this:

  Adviser adviser =
((ThesisProject)thing.getStudent().getMajor()).getAdviser();

then my approach won't work. (I'm not sure if PropertyResolver will work in
this case, but I suspect it will.)


-----
There are 10 kinds of people: Those who know binary and those who don't.
-- 
View this message in context: http://www.nabble.com/PropertyResolver-redesign-tp16495644p16576062.html
Sent from the Wicket - Dev mailing list archive at Nabble.com.


Re: PropertyResolver redesign

Posted by Maurice Marrink <ma...@gmail.com>.
I think Johan mentioned it but if memory serves me that was about
(de)serialization.

Maurice

On Sat, Apr 5, 2008 at 10:03 PM, James Carman
<ja...@carmanconsulting.com> wrote:
> Ok, I just wanted to make sure.  I know someone said something about
>  holding onto classes was a bad thing in models and stuff.  Holding
>  onto Methods is just as bad, I would assume.
>
>
>
>  On Sat, Apr 5, 2008 at 3:55 PM, Maurice Marrink <ma...@gmail.com> wrote:
>  > Well the cache is destroyed when the application is destroyed, so
>  >  technically there shouldn't be anything stopping class gc that i can
>  >  think off.
>  >
>  >  Maurice
>  >
>  >
>  >
>  >  On Sat, Apr 5, 2008 at 1:45 AM, James Carman <ja...@carmanconsulting.com> wrote:
>  >  > On Fri, Apr 4, 2008 at 4:03 PM, Matej Knopp <ma...@gmail.com> wrote:
>  >  >  > Hi,
>  >  >  >
>  >  >  >  first of all, are you sure property resolver is your bottleneck? I
>  >  >  >  really have hard time believing that.
>  >  >  >
>  >  >  >  PropertyResolver already caches the method instances based on target
>  >  >  >  object class.
>  >  >
>  >  >  Does this cause problems with class gc?
>  >  >
>  >
>

Re: PropertyResolver redesign

Posted by James Carman <ja...@carmanconsulting.com>.
Ok, I just wanted to make sure.  I know someone said something about
holding onto classes was a bad thing in models and stuff.  Holding
onto Methods is just as bad, I would assume.

On Sat, Apr 5, 2008 at 3:55 PM, Maurice Marrink <ma...@gmail.com> wrote:
> Well the cache is destroyed when the application is destroyed, so
>  technically there shouldn't be anything stopping class gc that i can
>  think off.
>
>  Maurice
>
>
>
>  On Sat, Apr 5, 2008 at 1:45 AM, James Carman <ja...@carmanconsulting.com> wrote:
>  > On Fri, Apr 4, 2008 at 4:03 PM, Matej Knopp <ma...@gmail.com> wrote:
>  >  > Hi,
>  >  >
>  >  >  first of all, are you sure property resolver is your bottleneck? I
>  >  >  really have hard time believing that.
>  >  >
>  >  >  PropertyResolver already caches the method instances based on target
>  >  >  object class.
>  >
>  >  Does this cause problems with class gc?
>  >
>

Re: PropertyResolver redesign

Posted by Maurice Marrink <ma...@gmail.com>.
Well the cache is destroyed when the application is destroyed, so
technically there shouldn't be anything stopping class gc that i can
think off.

Maurice

On Sat, Apr 5, 2008 at 1:45 AM, James Carman <ja...@carmanconsulting.com> wrote:
> On Fri, Apr 4, 2008 at 4:03 PM, Matej Knopp <ma...@gmail.com> wrote:
>  > Hi,
>  >
>  >  first of all, are you sure property resolver is your bottleneck? I
>  >  really have hard time believing that.
>  >
>  >  PropertyResolver already caches the method instances based on target
>  >  object class.
>
>  Does this cause problems with class gc?
>

Re: PropertyResolver redesign

Posted by James Carman <ja...@carmanconsulting.com>.
On Fri, Apr 4, 2008 at 4:03 PM, Matej Knopp <ma...@gmail.com> wrote:
> Hi,
>
>  first of all, are you sure property resolver is your bottleneck? I
>  really have hard time believing that.
>
>  PropertyResolver already caches the method instances based on target
>  object class.

Does this cause problems with class gc?

Re: PropertyResolver redesign

Posted by Matej Knopp <ma...@gmail.com>.
Hi,

first of all, are you sure property resolver is your bottleneck? I
really have hard time believing that.

PropertyResolver already caches the method instances based on target
object class.

Also your approach wouldn't work for things like "property1.property2"
where property 2 container type depends on result of property 1
evaluation.

-Matej

On Fri, Apr 4, 2008 at 9:03 PM, Miguel Munoz
<mm...@mainpointsystems.com> wrote:
>
>  Comrades,
>
>   After looking at the code for PropertyResolver, I find myself wondering if
>  there's a more efficent way to design it. It seems to me I could have more
>  efficient code if I could instantiate a resolver object which extracts the
>  getter and setter Methods on construction. Here's the situation I'm looking
>  at:
>
>  When displaying my data in a Repeating view, my sort code calls a Comparator
>  which looks something like this:
>
>   public int compare(Object o1, Object o2) {
>     String property = ... ;
>     Object val1 = PropertyResolver.getValue( property, o1 );
>     Object val2 = PropertyResolver.getValue( property, o2 );
>
>     return valueComparator.compare( val1, val2 );
>   }
>
>  For a given row, the sorter will call the getValue() method repeatedly, so I
>  want it to run quickly, but I can already see a problem:
>
>  The getValue() method needs to first extract a method from the property
>  name, then call that method to retrieve a value. That second step will
>  produce a different result for each row, but the first step, which extracts
>  the Method object, doesn't change from row to row, so it would be much more
>  efficent to do that part ahead of time. It seems to me that it shouldn't be
>  hard to write a new property resolver that I can instantiate, so the
>  extraction of the Method can be done ahead of time. In other words, I should
>  be able to write something like this:
>
>   PropertyResolver2 resolver = new PropertyResolver2("propertyName",
>  MyBean.class);
>
>  At this point, the resolver object will have a Method instance, or, for a
>  series of properties, an array of Methods. Then my comparator would look
>  like this:
>
>   public int compare(Object o1, Object o2) {  // o1 & o2 must be of class
>  MyBean.
>     String property = ... ;
>     Object val1 = resolver.getValue( o1 );
>     Object val2 = resolver.getValue( o2 );
>
>     return valueComparator.compare( val1, val2 );
>   }
>
>  The PropertyResolver2 constructor would already have the Method extracted,
>  so the comparator would execute much faster.
>
>  So here's my question: Is there any reason why the Wicket code doesn't do it
>  this way?
>
>  --
>  View this message in context: http://www.nabble.com/PropertyResolver-redesign-tp16495644p16495644.html
>  Sent from the Wicket - Dev mailing list archive at Nabble.com.
>
>



-- 
Resizable and reorderable grid components.
http://www.inmethod.com

Re: PropertyResolver redesign

Posted by James Carman <ja...@carmanconsulting.com>.
I have code that "records" a property invocation chain and bundles it
up into an object that can be "replayed" on a root object again (using
Commons Proxy's invocation recorder feature, not yet released).  It
has bindings for MVEL (which is the fastest currently), OGNL, Java
reflection, etc.  I don't think it's quite ready for "prime time" yet,
but you're more than welcome to it if you want it.

I would suggest you look into a simple MVEL-based implementation of
what you want.  MVEL is very fast and basically does what you're
looking for and it uses simple string expressions like Wicket does.

On Thu, May 8, 2008 at 11:10 PM, Miguel Munoz
<mm...@mainpointsystems.com> wrote:
>
>
> Eelco Hillenius wrote:
>>
>> One thing I'm wondering though is whether your more efficient version
>> still uses introspection? If that is the case, we would love to learn
>> what you did to make it run more efficiently. And - if you use
>> introspection - ... why use introspection when you need it to run very
>> efficiently (just write it out directly will run much faster).
>>
>> Eelco
>>
>>
>
> I am still using introspection because the current implementation uses
> property names specified in configuration files, although I may look into
> bypassing this. Here's how I suspect I get better efficiency: I instantiate
> distinct resolver objects for each property chain. When I instantiate one, I
> extract the getter's and setter's Method instances in the constructor. This
> means that when I set or retrieve the property, all I need to do is call
> Method.invoke(...). Since I'm sorting, I'm using the resolver for many
> different objects, so I don't have to retrieve the Method instance from the
> cache each time. (I suspect that's where I get most of the gains. HashMaps
> are generally pretty fast, but to improve sorting efficiency, I've found the
> best thing to do is speed up the individual comparisons.)
>
> (Also, since properties can be chained, I actually save an array of Getters.
> The longer the chain, the bigger the savings.)
>
> Earlier in this thread, Matej explained why this won't work for Wicket -- I
> guess your requirements are different from mine. I assume that a direct call
> to the property (e.g. item.getThing().getWidget().getVolume()) doesn't
> require anything to get cast to something else. I don't think my system
> would work if casting was needed, but I haven't tested that assumption.
>
> -----
> There are 10 kinds of people: Those who know binary and those who don't.
> --
> View this message in context: http://www.nabble.com/PropertyResolver-redesign-tp16495644p17140674.html
> Sent from the Wicket - Dev mailing list archive at Nabble.com.
>
>

Re: PropertyResolver redesign

Posted by Miguel Munoz <mm...@MainPointSystems.com>.

Eelco Hillenius wrote:
> 
> One thing I'm wondering though is whether your more efficient version
> still uses introspection? If that is the case, we would love to learn
> what you did to make it run more efficiently. And - if you use
> introspection - ... why use introspection when you need it to run very
> efficiently (just write it out directly will run much faster).
> 
> Eelco
> 
> 

I am still using introspection because the current implementation uses
property names specified in configuration files, although I may look into
bypassing this. Here's how I suspect I get better efficiency: I instantiate
distinct resolver objects for each property chain. When I instantiate one, I
extract the getter's and setter's Method instances in the constructor. This
means that when I set or retrieve the property, all I need to do is call
Method.invoke(...). Since I'm sorting, I'm using the resolver for many
different objects, so I don't have to retrieve the Method instance from the
cache each time. (I suspect that's where I get most of the gains. HashMaps
are generally pretty fast, but to improve sorting efficiency, I've found the
best thing to do is speed up the individual comparisons.)

(Also, since properties can be chained, I actually save an array of Getters.
The longer the chain, the bigger the savings.)

Earlier in this thread, Matej explained why this won't work for Wicket -- I
guess your requirements are different from mine. I assume that a direct call
to the property (e.g. item.getThing().getWidget().getVolume()) doesn't
require anything to get cast to something else. I don't think my system
would work if casting was needed, but I haven't tested that assumption.

-----
There are 10 kinds of people: Those who know binary and those who don't.
-- 
View this message in context: http://www.nabble.com/PropertyResolver-redesign-tp16495644p17140674.html
Sent from the Wicket - Dev mailing list archive at Nabble.com.


Re: PropertyResolver redesign

Posted by Eelco Hillenius <ee...@gmail.com>.
>  Yes, I hoped I was making it clear that this wasn't a real-world test, it was
>  a stress test to exaggerate the inefficiencies. This makes them easier to
>  measure. Stress tests have their place.

Yeah, I got that idea :-) However,

> While my code clearly wouldn't ever
>  see a case like this, we would see something similar: When many users are on
>  line, many threads will be sorting many smaller datasets. In this case, I
>  still want the sort to run as efficiently as possible. This is why I was
>  measuring the efficiency of the PropertyResolver, and why I still feel
>  there's a faster way to do this.

Fair enough. It's certainly something to keep in mind with any
flexible/ introspection based thing you do that there might be a cost.
If your scale is large enough, it might be a problem. That said, I
(and as is clear from that thread, 'we') don't think the efficiency of
PropertyResolver is a problem, and we have in fact optimized it as
much as we can within what we feel are reasonable limits (so without
losing flexibility).

> That said, I have accepted the Wicket team's claim that we shouldn't use
> PropertyResolver in our own code. I have developed my own resolver, which,
> as I expected, runs much more efficiently. (We tried OGNL and MVEL, but
> neither was really suitable.)

Funny enough, we started out using OGNL in early Wicket versions, but
decided to create our own resolver so that we could optimize it for
just our cases. What you're doing is another optimization for your
situation. :-)

One thing I'm wondering though is whether your more efficient version
still uses introspection? If that is the case, we would love to learn
what you did to make it run more efficiently. And - if you use
introspection - ... why use introspection when you need it to run very
efficiently (just write it out directly will run much faster).

Eelco

Re: PropertyResolver redesign

Posted by Miguel Munoz <mm...@MainPointSystems.com>.
Yes, I hoped I was making it clear that this wasn't a real-world test, it was
a stress test to exaggerate the inefficiencies. This makes them easier to
measure. Stress tests have their place. While my code clearly wouldn't ever
see a case like this, we would see something similar: When many users are on
line, many threads will be sorting many smaller datasets. In this case, I
still want the sort to run as efficiently as possible. This is why I was
measuring the efficiency of the PropertyResolver, and why I still feel
there's a faster way to do this. 

That said, I have accepted the Wicket team's claim that we shouldn't use
PropertyResolver in our own code. I have developed my own resolver, which,
as I expected, runs much more efficiently. (We tried OGNL and MVEL, but
neither was really suitable.)


Eelco Hillenius wrote:
> 
>>  The dataset I was sorting on was generated specifically to test the
>> speed of
>>  different comparators, and had a dataset of 100,000 elements, and I was
>>  sorting using the standard sort method from the collections package,
>>  accessed from Arrays.sort(). However, I now have data that times
>> individual
>>  calls of the Comparator's compare() method:
> 
> You would never load a data set of 100,000 elements (or even a 1,000
> for that matter) in Wicket models, simply because you wouldn't display
> that many components (your browser probably wouldn't be able to handle
> that for instance). Furthermore, even if you would find the need to do
> such sorting in-memory using Java, you would do that upfront, before
> arriving at the user interface layer.
> 
> Eelco
> 
> 


-----
There are 10 kinds of people: Those who know binary and those who don't.
-- 
View this message in context: http://www.nabble.com/PropertyResolver-redesign-tp16495644p17067504.html
Sent from the Wicket - Dev mailing list archive at Nabble.com.


Re: PropertyResolver redesign

Posted by Eelco Hillenius <ee...@gmail.com>.
>  The dataset I was sorting on was generated specifically to test the speed of
>  different comparators, and had a dataset of 100,000 elements, and I was
>  sorting using the standard sort method from the collections package,
>  accessed from Arrays.sort(). However, I now have data that times individual
>  calls of the Comparator's compare() method:

You would never load a data set of 100,000 elements (or even a 1,000
for that matter) in Wicket models, simply because you wouldn't display
that many components (your browser probably wouldn't be able to handle
that for instance). Furthermore, even if you would find the need to do
such sorting in-memory using Java, you would do that upfront, before
arriving at the user interface layer.

Eelco

Re: PropertyResolver redesign

Posted by Eelco Hillenius <ee...@gmail.com>.
>  It's very easy to say this, but the way Wicket is designed, we use property
>  names all over the place, so this forces us to use reflection when we sort.

There's nothing in Wicket's design that points to using property
models though. (Compound)PropertyModels are very convenient to use,
and that's why they show up at many examples, but we've been telling
the community for years that you have to be aware of a) the fact that
it breaks static typing and b) there is a slight performance impact.

Now, I don't think the performance impact is a big deal for most
people, as the bottlenecks are usually somewhere else (database access
and stuff). I know for sure it hasn't been a problem for any of the
projects I worked on with Wicket. But if it is for you, the best way
is to not use models that use introspection. There's nothing wrong or
anti-Wicket about using such models. It will mean that you'll get more
LOC, but it's good for performance, and as your models won't break
static typing, it'll be easier when it comes to refactoring and code
navigation (like finding out where certain objects are accessed).

Eelco

Re: PropertyResolver redesign

Posted by Eelco Hillenius <ee...@gmail.com>.
> While sorting is most often done by the database, I don't think Wicket's
> design should assume this is where sorting will be done. Wicket should leave
> that up to the application developers.

Sure, we don't assume that. However, we do assume that you're not
using models for massive sorting either. It's not their purpose to
support that, and PropertyModels in particular are written as
convenience over the cost of a slight performance penalty. That
penalty gets big with sufficient large data sets, but it is
unrealistic to have such large datasets in the first place (because
that won't render in browsers, and will be seriously bad for you
bandwidth usage).

Eelco

Re: PropertyResolver redesign

Posted by Eelco Hillenius <ee...@gmail.com>.
On Thu, May 8, 2008 at 3:46 PM, Miguel Munoz
<mm...@mainpointsystems.com> wrote:
>
> I'm not using PropertyResolver. I said so in a previous post. My point was
> that you shouldn't assume I don't know what I'm doing. Contrary to your
> previous post, there are valid reasons for sorting the data after retrieving
> it from the database.

Sure, I agree; there can be valid reasons for in memory sorting. And I
must have missed that you are not using property resolver... it's been
a long thread :-)

Eelco

Re: PropertyResolver redesign

Posted by Miguel Munoz <mm...@MainPointSystems.com>.
I'm not using PropertyResolver. I said so in a previous post. My point was
that you shouldn't assume I don't know what I'm doing. Contrary to your
previous post, there are valid reasons for sorting the data after retrieving
it from the database. 

(I'm sorry if I suggested that Wicket assumes that sorting will always be
done by the database. Wicket makes no such assumption. The assumption was
implicit in Igor's prior post, not in Wicket.)


igor.vaynberg wrote:
> 
> On Thu, May 8, 2008 at 2:43 PM, Miguel Munoz
> <mm...@mainpointsystems.com> wrote:
>>
>>  Igor is right in most cases. We do rely on the datastore to sort most of
>> the
>>  time, which, when it works, is the proper way to go. The reason the
>> issue
>>  comes up for us is that we may be sorting text data that changes from
>> one
>>  locale to another, and hence needs to return a sort order that is unique
>> for
>>  each Locale. For example, a field may consist of enumerated values, each
>> of
>>  which has localized display text. Sorting would produce one result in
>>  English and another in German. The database doesn't have the localized
>>  strings at all, and it shouldn't. So in this case, we need to delay the
>>  sorting until after we've retrieved the data. Then we sort using a
>>  Locale-sensitive Comparator.
>>
>>  While sorting is most often done by the database, I don't think Wicket's
>>  design should assume this is where sorting will be done. Wicket should
>> leave
>>  that up to the application developers.
> 
> wicket's design makes no assumptions about how the data is stored,
> sorted, or retrieved. it simply asks you for an iterator(). if you
> choose to sort your data in memory using reflection then you take the
> performance hit. if propertyresolver doesnt work for you then just
> dont use it - wicket doesnt force you in any way to do so. there are
> plenty of other libs out there that can do better for this particular
> usecase.
> 
> -igor
> 
>>
>>
>>
>>  igor.vaynberg wrote:
>>  >
>>  > erm. what?
>>  >
>>  > you should be sorting your data inside the datastore that stores it.
>>  > eg if you are using the database you dont pull out all rows and sort
>>  > them in memory, you use the database to do the sorting. dont know what
>>  > kind of datastore you are using, but you are definetely doing
>>  > something funny. furthermore, there is nothing in wicket's design that
>>  > dictates how your application should be structured, how it should
>>  > access the datastore, or how it should do its sorting or any other
>>  > kind of data handling.
>>  >
>>  > -igor
>>  >
>>  >
>>
>>
>>
>> -----
>>  There are 10 kinds of people: Those who know binary and those who don't.
>>  --
>>  View this message in context:
>> http://www.nabble.com/PropertyResolver-redesign-tp16495644p17137042.html
>>
>>
>> Sent from the Wicket - Dev mailing list archive at Nabble.com.
>>
>>
> 
> 


-----
There are 10 kinds of people: Those who know binary and those who don't.
-- 
View this message in context: http://www.nabble.com/PropertyResolver-redesign-tp16495644p17138176.html
Sent from the Wicket - Dev mailing list archive at Nabble.com.


Re: PropertyResolver redesign

Posted by Igor Vaynberg <ig...@gmail.com>.
On Thu, May 8, 2008 at 2:43 PM, Miguel Munoz
<mm...@mainpointsystems.com> wrote:
>
>  Igor is right in most cases. We do rely on the datastore to sort most of the
>  time, which, when it works, is the proper way to go. The reason the issue
>  comes up for us is that we may be sorting text data that changes from one
>  locale to another, and hence needs to return a sort order that is unique for
>  each Locale. For example, a field may consist of enumerated values, each of
>  which has localized display text. Sorting would produce one result in
>  English and another in German. The database doesn't have the localized
>  strings at all, and it shouldn't. So in this case, we need to delay the
>  sorting until after we've retrieved the data. Then we sort using a
>  Locale-sensitive Comparator.
>
>  While sorting is most often done by the database, I don't think Wicket's
>  design should assume this is where sorting will be done. Wicket should leave
>  that up to the application developers.

wicket's design makes no assumptions about how the data is stored,
sorted, or retrieved. it simply asks you for an iterator(). if you
choose to sort your data in memory using reflection then you take the
performance hit. if propertyresolver doesnt work for you then just
dont use it - wicket doesnt force you in any way to do so. there are
plenty of other libs out there that can do better for this particular
usecase.

-igor

>
>
>
>  igor.vaynberg wrote:
>  >
>  > erm. what?
>  >
>  > you should be sorting your data inside the datastore that stores it.
>  > eg if you are using the database you dont pull out all rows and sort
>  > them in memory, you use the database to do the sorting. dont know what
>  > kind of datastore you are using, but you are definetely doing
>  > something funny. furthermore, there is nothing in wicket's design that
>  > dictates how your application should be structured, how it should
>  > access the datastore, or how it should do its sorting or any other
>  > kind of data handling.
>  >
>  > -igor
>  >
>  >
>
>
>
> -----
>  There are 10 kinds of people: Those who know binary and those who don't.
>  --
>  View this message in context: http://www.nabble.com/PropertyResolver-redesign-tp16495644p17137042.html
>
>
> Sent from the Wicket - Dev mailing list archive at Nabble.com.
>
>

Re: PropertyResolver redesign

Posted by Miguel Munoz <mm...@MainPointSystems.com>.
Igor is right in most cases. We do rely on the datastore to sort most of the
time, which, when it works, is the proper way to go. The reason the issue
comes up for us is that we may be sorting text data that changes from one
locale to another, and hence needs to return a sort order that is unique for
each Locale. For example, a field may consist of enumerated values, each of
which has localized display text. Sorting would produce one result in
English and another in German. The database doesn't have the localized
strings at all, and it shouldn't. So in this case, we need to delay the
sorting until after we've retrieved the data. Then we sort using a
Locale-sensitive Comparator.

While sorting is most often done by the database, I don't think Wicket's
design should assume this is where sorting will be done. Wicket should leave
that up to the application developers.


igor.vaynberg wrote:
> 
> erm. what?
> 
> you should be sorting your data inside the datastore that stores it.
> eg if you are using the database you dont pull out all rows and sort
> them in memory, you use the database to do the sorting. dont know what
> kind of datastore you are using, but you are definetely doing
> something funny. furthermore, there is nothing in wicket's design that
> dictates how your application should be structured, how it should
> access the datastore, or how it should do its sorting or any other
> kind of data handling.
> 
> -igor
> 
> 


-----
There are 10 kinds of people: Those who know binary and those who don't.
-- 
View this message in context: http://www.nabble.com/PropertyResolver-redesign-tp16495644p17137042.html
Sent from the Wicket - Dev mailing list archive at Nabble.com.


Re: PropertyResolver redesign

Posted by Igor Vaynberg <ig...@gmail.com>.
erm. what?

you should be sorting your data inside the datastore that stores it.
eg if you are using the database you dont pull out all rows and sort
them in memory, you use the database to do the sorting. dont know what
kind of datastore you are using, but you are definetely doing
something funny. furthermore, there is nothing in wicket's design that
dictates how your application should be structured, how it should
access the datastore, or how it should do its sorting or any other
kind of data handling.

-igor


On Mon, May 5, 2008 at 12:17 PM, Miguel Munoz
<mm...@mainpointsystems.com> wrote:
>
>
>  igor.vaynberg wrote:
>  >
>  > hmm, right
>  >
>  > so you are running an upwards of 100K reflection lookups for the first
>  > test, and upwards of 300K lookups in the second test. and you want
>  > this to be performant?
>  >
>  No. But I do want my sorts to run as efficiently as possible.
>
>
>
>  igor.vaynberg wrote:
>  >
>  > sorry, but there are much better ways to sort data that do not involve
>  > reflection.
>  >
>
>  It's very easy to say this, but the way Wicket is designed, we use property
>  names all over the place, so this forces us to use reflection when we sort.
>
>
>  -----
>  There are 10 kinds of people: Those who know binary and those who don't.
>  --
>  View this message in context: http://www.nabble.com/PropertyResolver-redesign-tp16495644p17068896.html
>
>
> Sent from the Wicket - Dev mailing list archive at Nabble.com.
>
>

Re: PropertyResolver redesign

Posted by Miguel Munoz <mm...@MainPointSystems.com>.

igor.vaynberg wrote:
> 
> hmm, right
> 
> so you are running an upwards of 100K reflection lookups for the first
> test, and upwards of 300K lookups in the second test. and you want
> this to be performant?
> 
No. But I do want my sorts to run as efficiently as possible.


igor.vaynberg wrote:
> 
> sorry, but there are much better ways to sort data that do not involve
> reflection. 
> 

It's very easy to say this, but the way Wicket is designed, we use property
names all over the place, so this forces us to use reflection when we sort.

-----
There are 10 kinds of people: Those who know binary and those who don't.
-- 
View this message in context: http://www.nabble.com/PropertyResolver-redesign-tp16495644p17068896.html
Sent from the Wicket - Dev mailing list archive at Nabble.com.


Re: PropertyResolver redesign

Posted by Igor Vaynberg <ig...@gmail.com>.
hmm, right

so you are running an upwards of 100K reflection lookups for the first
test, and upwards of 300K lookups in the second test. and you want
this to be performant?

sorry, but there are much better ways to sort data that do not involve
reflection. so perhaps when you come up with a realistic usecase that
shows property resolver is slow for framework usecases we will
consider tweaking it in some way.

-igor

On Tue, Apr 8, 2008 at 4:50 PM, Miguel Munoz
<mm...@mainpointsystems.com> wrote:
>
>
>  igor.vaynberg wrote:
>  >
>  >
>  >
>  > i still dont understand how you use propertyresolver for "sorting". is
>  > it that the dynamic string here represents a method that returns the
>  > value you sort on? to get 800ms, how big is your dataset that you are
>  > sorting, and how are you sorting it?
>  >
>  >
>
>  I'm using the Comparator that I put in my original post, which looks like
>
> this:
>
>   public int compare(Object o1, Object o2) {
>     String property = ... ;
>     Object val1 = PropertyResolver.getValue( property, o1 );
>     Object val2 = PropertyResolver.getValue( property, o2 );
>
>     return valueComparator.compare( val1, val2 );
>   }
>
>  The valueComparator instance in the last line determines if the objects
>  implement Comparable, which they do, then casts them and calls the
>  compareTo() method. (In my timing results below, I simplify this by casting
>  them to a CollationKey without doing any tests.)
>
>  The dataset I was sorting on was generated specifically to test the speed of
>  different comparators, and had a dataset of 100,000 elements, and I was
>  sorting using the standard sort method from the collections package,
>  accessed from Arrays.sort(). However, I now have data that times individual
>  calls of the Comparator's compare() method:
>
>  Without property chaining, PropertyResolver took 16 µs, while my class took
>  7 µs. (I hope the mu character comes through here. That's supposed to be
>  microseconds.)
>
>  When I tried chaining three properties together, I got 38 µs with
>  PropertyResolver, and 12 µs with my approach. (I only timed getters.)
>
>  In both these cases, I'm comparing strings of only one character, so most of
>  the effort is spent resolving the property chains. Also, in each approach,
>  I'm throwing out the first use, since it spends part of its time loading
>  classes and caching Methods.
>
>
>
>
>  -----
>  There are 10 kinds of people: Those who know binary and those who don't.
>  --
>  View this message in context: http://www.nabble.com/PropertyResolver-redesign-tp16495644p16576480.html
>
>
> Sent from the Wicket - Dev mailing list archive at Nabble.com.
>
>

Re: PropertyResolver redesign

Posted by Miguel Munoz <mm...@MainPointSystems.com>.

igor.vaynberg wrote:
> 
> 
> 
> i still dont understand how you use propertyresolver for "sorting". is
> it that the dynamic string here represents a method that returns the
> value you sort on? to get 800ms, how big is your dataset that you are
> sorting, and how are you sorting it?
> 
> 

I'm using the Comparator that I put in my original post, which looks like
this:

  public int compare(Object o1, Object o2) {
    String property = ... ;
    Object val1 = PropertyResolver.getValue( property, o1 );
    Object val2 = PropertyResolver.getValue( property, o2 );

    return valueComparator.compare( val1, val2 );
  }

The valueComparator instance in the last line determines if the objects
implement Comparable, which they do, then casts them and calls the
compareTo() method. (In my timing results below, I simplify this by casting
them to a CollationKey without doing any tests.) 

The dataset I was sorting on was generated specifically to test the speed of
different comparators, and had a dataset of 100,000 elements, and I was
sorting using the standard sort method from the collections package,
accessed from Arrays.sort(). However, I now have data that times individual
calls of the Comparator's compare() method:

Without property chaining, PropertyResolver took 16 µs, while my class took
7 µs. (I hope the mu character comes through here. That's supposed to be
microseconds.)

When I tried chaining three properties together, I got 38 µs with
PropertyResolver, and 12 µs with my approach. (I only timed getters.)

In both these cases, I'm comparing strings of only one character, so most of
the effort is spent resolving the property chains. Also, in each approach,
I'm throwing out the first use, since it spends part of its time loading
classes and caching Methods.



-----
There are 10 kinds of people: Those who know binary and those who don't.
-- 
View this message in context: http://www.nabble.com/PropertyResolver-redesign-tp16495644p16576480.html
Sent from the Wicket - Dev mailing list archive at Nabble.com.


Re: PropertyResolver redesign

Posted by Igor Vaynberg <ig...@gmail.com>.
like i said in my original email: PropertyResolver is optimized for
our usecases, it delivers awesome performance - but only when used for
those usecases.

you are never meant to use propertyresolver yourself. i will add that
to the javadoc.

i still dont understand how you use propertyresolver for "sorting". is
it that the dynamic string here represents a method that returns the
value you sort on? to get 800ms, how big is your dataset that you are
sorting, and how are you sorting it?

-igor


On Mon, Apr 7, 2008 at 2:25 PM, Miguel Munoz
<mm...@mainpointsystems.com> wrote:
>
>
>  igor.vaynberg wrote:
>  >
>  > PropertyResolver is not meant for your direct consumption. it is
>  > something we use for our own purposes inside wicket.
>  >
>
>  This is a bit surprising, since the Javadocs provide a very friendly
>  explanation of how to use the class. If it's not intended for us, maybe the
>  Javadocs should say so.
>
>
>
>  igor.vaynberg wrote:
>  >
>  > i am not sure we are willing to change how it works as it gives us no
>  > benefit, the class as is right now is optimized for our specific
>  > usecases.
>  >
>  >
>
>  If you like, I will present more detailed performance data when I have it.
>  While I'm happy to switch to a different library, I would still like to know
>  that Wicket performs as fast as possible.
>
>
>
>  igor.vaynberg wrote:
>  >
>  > you can use a library like mvel/ognl to perform dynamic evaluations
>  > that are for your needs. i hear mvel is pretty good.
>  >
>  >
>
>  Are you saying here that PropertyResolver shouldn't be used for
>  performance-critical tasks like sorting? If so, you might want to say so,
>  again, in the javadocs. (You should also suggest mvel.) That said, I'm happy
>  to switch to mvel if that will give me the performance I want.
>
>
>
>
>  -----
>  There are 10 kinds of people: Those who know binary and those who don't.
>  --
>  View this message in context: http://www.nabble.com/PropertyResolver-redesign-tp16495644p16540848.html
>
>
> Sent from the Wicket - Dev mailing list archive at Nabble.com.
>
>

Re: PropertyResolver redesign

Posted by Miguel Munoz <mm...@MainPointSystems.com>.

igor.vaynberg wrote:
> 
> PropertyResolver is not meant for your direct consumption. it is
> something we use for our own purposes inside wicket.
> 

This is a bit surprising, since the Javadocs provide a very friendly
explanation of how to use the class. If it's not intended for us, maybe the
Javadocs should say so.


igor.vaynberg wrote:
> 
> i am not sure we are willing to change how it works as it gives us no
> benefit, the class as is right now is optimized for our specific
> usecases.
> 
> 

If you like, I will present more detailed performance data when I have it.
While I'm happy to switch to a different library, I would still like to know
that Wicket performs as fast as possible. 


igor.vaynberg wrote:
> 
> you can use a library like mvel/ognl to perform dynamic evaluations
> that are for your needs. i hear mvel is pretty good.
> 
> 

Are you saying here that PropertyResolver shouldn't be used for
performance-critical tasks like sorting? If so, you might want to say so,
again, in the javadocs. (You should also suggest mvel.) That said, I'm happy
to switch to mvel if that will give me the performance I want.



-----
There are 10 kinds of people: Those who know binary and those who don't.
-- 
View this message in context: http://www.nabble.com/PropertyResolver-redesign-tp16495644p16540848.html
Sent from the Wicket - Dev mailing list archive at Nabble.com.


Re: PropertyResolver redesign

Posted by Igor Vaynberg <ig...@gmail.com>.
PropertyResolver is not meant for your direct consumption. it is
something we use for our own purposes inside wicket.

i am not sure we are willing to change how it works as it gives us no
benefit, the class as is right now is optimized for our specific
usecases.

you can use a library like mvel/ognl to perform dynamic evaluations
that are for your needs. i hear mvel is pretty good.

of course, this is just my opinion, maybe other core devs have others.

-igor


On Fri, Apr 4, 2008 at 12:03 PM, Miguel Munoz
<mm...@mainpointsystems.com> wrote:
>
>  Comrades,
>
>   After looking at the code for PropertyResolver, I find myself wondering if
>  there's a more efficent way to design it. It seems to me I could have more
>  efficient code if I could instantiate a resolver object which extracts the
>  getter and setter Methods on construction. Here's the situation I'm looking
>  at:
>
>  When displaying my data in a Repeating view, my sort code calls a Comparator
>  which looks something like this:
>
>   public int compare(Object o1, Object o2) {
>     String property = ... ;
>     Object val1 = PropertyResolver.getValue( property, o1 );
>     Object val2 = PropertyResolver.getValue( property, o2 );
>
>     return valueComparator.compare( val1, val2 );
>   }
>
>  For a given row, the sorter will call the getValue() method repeatedly, so I
>  want it to run quickly, but I can already see a problem:
>
>  The getValue() method needs to first extract a method from the property
>  name, then call that method to retrieve a value. That second step will
>  produce a different result for each row, but the first step, which extracts
>  the Method object, doesn't change from row to row, so it would be much more
>  efficent to do that part ahead of time. It seems to me that it shouldn't be
>  hard to write a new property resolver that I can instantiate, so the
>  extraction of the Method can be done ahead of time. In other words, I should
>  be able to write something like this:
>
>   PropertyResolver2 resolver = new PropertyResolver2("propertyName",
>  MyBean.class);
>
>  At this point, the resolver object will have a Method instance, or, for a
>  series of properties, an array of Methods. Then my comparator would look
>  like this:
>
>   public int compare(Object o1, Object o2) {  // o1 & o2 must be of class
>  MyBean.
>     String property = ... ;
>     Object val1 = resolver.getValue( o1 );
>     Object val2 = resolver.getValue( o2 );
>
>     return valueComparator.compare( val1, val2 );
>   }
>
>  The PropertyResolver2 constructor would already have the Method extracted,
>  so the comparator would execute much faster.
>
>  So here's my question: Is there any reason why the Wicket code doesn't do it
>  this way?
>
>  --
>  View this message in context: http://www.nabble.com/PropertyResolver-redesign-tp16495644p16495644.html
>  Sent from the Wicket - Dev mailing list archive at Nabble.com.
>
>

Re: PropertyResolver redesign

Posted by Johan Compagner <jc...@gmail.com>.
if property is just one property (not a dot.dot.dot thing)
then i cant believe that the property resolver is a big factor because it
very fast resolves to the right cached invoker.

It is not easy to open this up because of the dot.dot.dot thing because for
that it is a recursive call on itself that
does the object resolving on its way,.

johan


On Fri, Apr 4, 2008 at 9:03 PM, Miguel Munoz <mm...@mainpointsystems.com>
wrote:

>
> Comrades,
>
>  After looking at the code for PropertyResolver, I find myself wondering
> if
> there's a more efficent way to design it. It seems to me I could have more
> efficient code if I could instantiate a resolver object which extracts the
> getter and setter Methods on construction. Here's the situation I'm
> looking
> at:
>
> When displaying my data in a Repeating view, my sort code calls a
> Comparator
> which looks something like this:
>
>  public int compare(Object o1, Object o2) {
>    String property = ... ;
>    Object val1 = PropertyResolver.getValue( property, o1 );
>    Object val2 = PropertyResolver.getValue( property, o2 );
>
>    return valueComparator.compare( val1, val2 );
>  }
>
> For a given row, the sorter will call the getValue() method repeatedly, so
> I
> want it to run quickly, but I can already see a problem:
>
> The getValue() method needs to first extract a method from the property
> name, then call that method to retrieve a value. That second step will
> produce a different result for each row, but the first step, which
> extracts
> the Method object, doesn't change from row to row, so it would be much
> more
> efficent to do that part ahead of time. It seems to me that it shouldn't
> be
> hard to write a new property resolver that I can instantiate, so the
> extraction of the Method can be done ahead of time. In other words, I
> should
> be able to write something like this:
>
>  PropertyResolver2 resolver = new PropertyResolver2("propertyName",
> MyBean.class);
>
> At this point, the resolver object will have a Method instance, or, for a
> series of properties, an array of Methods. Then my comparator would look
> like this:
>
>  public int compare(Object o1, Object o2) {  // o1 & o2 must be of class
> MyBean.
>    String property = ... ;
>    Object val1 = resolver.getValue( o1 );
>    Object val2 = resolver.getValue( o2 );
>
>    return valueComparator.compare( val1, val2 );
>  }
>
> The PropertyResolver2 constructor would already have the Method extracted,
> so the comparator would execute much faster.
>
> So here's my question: Is there any reason why the Wicket code doesn't do
> it
> this way?
>
> --
> View this message in context:
> http://www.nabble.com/PropertyResolver-redesign-tp16495644p16495644.html
> Sent from the Wicket - Dev mailing list archive at Nabble.com.
>
>