On Jul 15, 2010, at 9:41 AM, Howard Lewis Ship wrote:

> As soon as we have the new Confluence documentation system up and
> running, I'll be pushing to get an alpha out, switch to beta
> development (bug fixing) and documentation.  I think it'll be a short
> beta ... but we do have an awful lot of little bugs that could use
> some TLC.

  Yeah, so it seems to me like 5.2 has some good stuff in it that should get 
pushed out, like running easily under GlassFish 3.x, etc. But now you want to 
rip up the floorboards to implement a performance optimization. But ripping up 
the floorboards is making me nervous. 

  Specifically, the thing that bothers me about this is your last paragraph:

>    Back to pooling: how is this going to affect performance? That's an open 
> question, and putting together a performance testing environment is another 
> task at the top of my list. My suspicion is that the new overhead will not 
> make a visible difference for small applications (dozens of pages, reasonable 
> number of concurrent users) ... but for high end sites (hundreds of pages, 
> large numbers of concurrent users) the avoidance of pooling and page 
> construction will make a big difference!

  Ok, this is all in the name of performance memory and otherwise, but the code 
is being developed before the test structure is implemented? Premature 
Optimization, its worse than Premature Ejaculation! Measure, then optimize!

  You're completely changing the architecture to something you are assuming 
will be faster, but this new architecture seems like it will need lots of small 
tweaks. For instance, if you're using a Map to store the values, it might make 
a lot of performance sense to pre-allocate the right capacity for the Map, 
because after scanning the page, you'll know how many fields you have to store. 
That means you need to collect meta info about each page somewhere. 

  Other little things: Your key is: String key = 
String.format("UnclaimedFieldWorker:%s/%s",

  That means that the beginning of the key is always the same. Even though the 
lookup is probably a hash, it would be better to have the most widely varying 
data in the front, not the back. Even better might be not to use a Map at all, 
but rather map field names to indexes as you go and use an Array; though that 
would have worse memory performance if you have 100 default values and 1 custom 
value. 

  All of that tells me that this is something that should be done in a branch. 
BUT...

  I'm sure you feel beat up right now. I don't want you to feel that way. 

 So let me also propose the counter argument, which if I understand correctly, 
that persistent fields already work that way, so really this is extending 
regular fields to work the same way as persistent fields. If that's true, then 
I'm only half right:

     1. I was wrong:   To quote Thiago: "From the Tapestry user point of view, 
it's a completely internal, transparent change."

     2. I was right: We should come up with some performance tests for this. 
That might just be a matter of asking the guy who had been doing Tapestry 
capacity testing to run his tests against trunk. 

 Pierce
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscr...@tapestry.apache.org
For additional commands, e-mail: users-h...@tapestry.apache.org

Reply via email to