Thanks for the input Simone. Perhaps I can clarify something. The property access stuff was *not* designed to ignore performance, quite the contrary. It is a level of indirection that was added to allow multiple concerns to be addressed, one major one being performance.
Now I am ready to admit that perhaps it was a mistake to make the default property access method the slow one. I would be all for making the default one fast and the one to be used by the renderer. But also to my defense this was discussed on the mailing list, designed on a wiki page, etc.. To be blunt the people developing on 2.3.x did not care since it was happening on trunk. I agree that it is important to take performance into account when coming up with a design. But I disagree that it should be the driving force. I have found that pre-optimization usually leads to one wasting cycles thinking about what *might* happen. I have found ( and this is just my opinion of course ), that it is a far more effective use of time to analyze after the fact with a profiler to find out what *actually* is happening and then optimize. If the original design was good enough one should be able to fix any bottlenecks that arise. If not then it needs to be readdressed. -Justin Simone Giannecchini wrote: > Ciao guys, > just wanted to throw my 2 cents here, without the will to seem polemic. > >> I admit, this stuff needs some performance tuning, but that doesn;t mean >> it needs to be "fixed". It was not designed with performance in mind. > > That's really bad IMHO. Bad design is bad, good design is good but only if > it is backed by > good performace, otherwise it is just a theoretical proof of concept and > it's of no no help for users > and not even for the others developers. > > >> The wfs 1.1. spec requires us to support xpath against features. This by >> design is going to not be the most efficient way of property access. >> Which is why we went to the trouble of breaking out an interface and >> extension point. So I think a bit more care is warranted. > > Good to know that we are leaning towards 1.1, but hey, what about WMS? I > don't think > we should just forget about having maximum performances for it just because > we want WFS 1.1 out quicly. > > I think before doing such changes, whatever the reason pushing them out is, > we should be 120% (yeah 120% :-) ) > that we are not going to impact other code and especially performances, > because there is nothing that impact > users experience more than performance, at least IMHO. So, yeah,a bit more > care should be warranted but before > committing the changes the first time. > >> Instead of "fixing" the code that is there, I would rather you implement >> a different property accessor strictly for the renderer ( settable via a >> hint ) whose prime concern is performance. > > I am sorry, but I do not think this is fair, especially with regards to > Andrea. To me, it sounds like, > "I did what I had to do, if it does not work for you go ahead and fix it, > but the way I tell you" :-). > Andrea (and myself as well) has spent days and nights profiling GeoServer > looking for performance problems, he did a > wonderful job spotting hidden problems, and now we drop in a lot of changes > with a huge performance loss? > I understand his frustrations on this issue with the renderer. I am the > living proof that adding things to a "live" codebase > like GeoTools can cause glitches since I regularly cause problems, but I > think that ensuring performances NEVER degrade is > essential and whatever changes we commit we should ensure so or simply we > should reject the changes until we does ensure so. > > I think that a little bit of care could be used before doing big changes (I > am talking to myself as well here :-) ) > rather than counting on someone else to fix things. > > Note that my intent is not to be polemic but just to require from al a > little bit more of attention before doing big changes. > > Ciao, > Simone. > > Btw, by reading maling-list' message what's more core than renderer > performances??? > > ------------------------------------------------------- > Eng. Simone Giannecchini > > President/CEO GeoSolutions > http://www.geo-solutions.it > Via Carignoni 51 > 550141 Camaiore (LU) > Italy > Mobile: +39 333 81 28928 > ------------------------------------------------------- > ----- Original Message ----- > From: "Justin Deoliveira" <[EMAIL PROTECTED]> > To: "Andrea Aime" <[EMAIL PROTECTED]> > Cc: "Jody Garnett" <[EMAIL PROTECTED]>; "Geotools-Devel list" > <[email protected]> > Sent: Monday, February 12, 2007 1:37 AM > Subject: Re: [Geotools-devel] PropertyAccess subsystem killing our > rendering > performance > > >> Hi Andrea, >> >> I am kind of puzzled why I am not seeing any patches for any of this, or >> at least a wait for some feedback. This is pretty core stuff you are >> changing, and I would like a chance to comment on it before it gets >> committed. I believe this was part of the new process we agreed on, no? >> >> I admit, this stuff needs some performance tuning, but that doesn;t mean >> it needs to be "fixed". It was not designed with performance in mind. >> The wfs 1.1. spec requires us to support xpath against features. This by >> design is going to not be the most efficient way of property access. >> Which is why we went to the trouble of breaking out an interface and >> extension point. So I think a bit more care is warranted. >> >> Instead of "fixing" the code that is there, I would rather you implement >> a different property accessor strictly for the renderer ( settable via a >> hint ) whose prime concern is performance. >> >> -Justin >> >> >> Andrea Aime wrote: >>> Andrea Aime ha scritto: >>>> History goes on, >>>> fixed the two above, rendering time is down to 40 seconds >>>> on java 1.4 and 6 seconds on java 1.5, which makes me think >>>> we're allocating too many objects ... investigating... >>> >>> Oh well, I don't really know how to improve it more by quick >>> tricks, some deeper thougth is needed here. >>> What I can see, is that the worst offenders now are >>> Class.isInstance(xxx) in the Value class, Class.isAssignableFrom >>> in SimpleFeaturePropertyAccessorFactory, and the two togher >>> sum up to twice the time spent actually drawing with SunGraphics2D.draw >>> (this is horrible). >>> >>> An interesting note is that the same rendering code you >>> can get on the user mailing list now takes: >>> * 24 seconds on jdk 1.4 >>> * 5.2 seconds on jdk 1.5 >>> * 3.8 seconds on jdk 1.6 >>> >>> Well, at least Sun is helping here :-) >>> Cheers >>> Andrea >>> >>> >>> >> >> >> -- >> Justin Deoliveira >> [EMAIL PROTECTED] >> The Open Planning Project >> http://topp.openplans.org >> >> ------------------------------------------------------------------------- >> Using Tomcat but need to do more? Need to support web services, security? >> Get stuff done quickly with pre-integrated technology to make your job >> easier. >> Download IBM WebSphere Application Server v.1.0.1 based on Apache >> Geronimo >> http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642 >> _______________________________________________ >> Geotools-devel mailing list >> [email protected] >> https://lists.sourceforge.net/lists/listinfo/geotools-devel > > !DSPAM:1004,45cfc443140841425493344! > -- Justin Deoliveira [EMAIL PROTECTED] The Open Planning Project http://topp.openplans.org ------------------------------------------------------------------------- Using Tomcat but need to do more? Need to support web services, security? Get stuff done quickly with pre-integrated technology to make your job easier. Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642 _______________________________________________ Geotools-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/geotools-devel
