You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cocoon.apache.org by Antonio Gallardo <ag...@agssa.net> on 2005/05/28 03:20:48 UTC
Re: svn commit: r178778 -
/cocoon/branches/BRANCH_2_1_X/src/blocks/forms/java/org/apache/cocoon/
forms/datatype/DynamicSelectionList.java
/cocoon/branches/BRANCH_2_1_X/src/blocks/forms/java/org/apache/cocoon/
forms/formmodel/Field.java /cocoon/branches/BRANCH_2_1_X/status.xml
Hi Joerg:
I am glad to see you active on the list again. As I told you in GT2004, I
like your radar over my work. :-)
[comments below]
On Vie, 27 de Mayo de 2005, 19:02, Joerg Heinicke dijo:
> On 27.05.2005 16:05, antonio@apache.org wrote:
>> Author: antonio
>> Date: Fri May 27 07:05:02 2005
>> New Revision: 178778
>>
>> URL: http://svn.apache.org/viewcvs?rev=178778&view=rev
>> Log:
>> Cforms block: Improved dynamic selection list performance inside
>> repeaters.
>
> Sorry, but I absolutely don't like it for the following reasons:
It is OK. :-)
I clearly understand your concerns and I agree with them. As I told
before, I am a newbie inside cform code. Sorry.
My idea was to rewrite this as clean as posible over the weekend. But I
was in a hurry and wanted to know if the idea will work at all. This is
also why I don't committed to the trunk. To allow other people test if
this works.
I only got one reply, I didn't knew if the community was in favor of the
change (I assumed a lazy approval). This code was done mostly as a proof
of concept. To see if this really will improve the performance.
After all, I think there is a good news: The proof was successful. While
Using 3 combos inside a repeater I can see the code runs 2 to 3 times
faster than before. When the repeater grows, there is almost no
performance penality now.
Now, I am also thinking to improve now other pieces, for example, there is
another big time penality inside selection-lists. Again when we are inside
a repeaters. When we change the value of a selection-list using something
like that:
<fd:on-value-changed>
<fd:javascript>
myWidget.setSelectionList("cocoon:/mySelectionList.data?id=" + value);
</fd:javascript>
</fd:on-value-changed>
Here again, the current behavior is to reread the data. And I noted in
some cases is posible the same combo will be there over and over so a
cache improvement will save a lot of time again. But this is another
story. First, I will try to fix the current one.
> 1. You introduced a dependency on the class DynamicSelectionList into
> Field.java where before was only the interface SelectionList.
>
> 2. You introduced a dependency on the classes Repeater and RepeaterRow.
>
> 3. You mixed builder concerns with datatype concerns (copying code from
> DefaultSelectionListBuilder to DynamicSelectionList).
>
> 4. The implementation of DynamicSelectionList got much more complex,
> where I don't see a reason why? What's the difference between building a
> selection list in generateSaxFragment() and in prepareCache()? 90 lines
> of code just for caching?
Agreed. Totally ugly code. I just wanted to touch the less posible files
to see if this works.
> Requirements:
>
> 1. It must be absolute transparent to Field.java (above issues 1 + 2).
>
> 2. No code duplication in sense of copy & paste (above issue 3).
>
> 3. Simple code! No code duplication in sense of having two parts of code
> doing the same (above issue 4).
>
> Solution/suggestion:
>
> What for do we have source validities? Wouldn't help a
> SelecionListHandler storing the SAX events in a SAX buffer and
> resaxifying the events if source validity is still valid and wouldn't it
> fulfill all requirements?
Thanks again for the tip. I will try to do this over this weekend.
Best Regards,
Antonio Gallardo
Re: svn commit: r178778 - /cocoon/branches/BRANCH_2_1_X/src/blocks/forms/java/org/apache/cocoon/
forms/datatype/DynamicSelectionList.java /cocoon/branches/BRANCH_2_1_X/src/blocks/forms/java/org/apache/cocoon/
forms/formmodel/Field.java /cocoon/branches/BRANCH_2_1_X/status.xml
Posted by Sylvain Wallez <sy...@apache.org>.
Antonio Gallardo wrote:
>I only got one reply, I didn't knew if the community was in favor of the change (I assumed a lazy approval).
>
Sorry Antonio, I was a way lately and hadn't the time to answer your RT.
There is a large range of greys between static and dynamic selection
lists through the "type" attribute of <fd:selection-list>. I'll answer
your RT with some proposals in the coming hours.
Sylvain
--
Sylvain Wallez Anyware Technologies
http://apache.org/~sylvain http://anyware-tech.com
Apache Software Foundation Member Research & Technology Director
Re: svn commit: r178778 -
/cocoon/branches/BRANCH_2_1_X/src/blocks/forms/java/org/apache/cocoon/
forms/datatype/DynamicSelectionList.java
/cocoon/branches/BRANCH_2_1_X/src/blocks/forms/java/org/apache/cocoon/
forms/formmodel/Field.java
/cocoon/branches/BRANCH_2_1_X/status.xml
Posted by Antonio Gallardo <ag...@agssa.net>.
On Sab, 28 de Mayo de 2005, 10:51, Joerg Heinicke dijo:
> Now I'm going to read your initial RT and Sylvain's answer :-)
Yep. Please join us in the Sylvain reply to this thread. I think there we
already found a good solution to this problem. I am going to revert the
changes I did, since they don't cover all the posible situations. In
short, it was buggy.
Best Regards,
Antonio Gallardo.
Re: svn commit: r178778 - /cocoon/branches/BRANCH_2_1_X/src/blocks/forms/java/org/apache/cocoon/
forms/datatype/DynamicSelectionList.java /cocoon/branches/BRANCH_2_1_X/src/blocks/forms/java/org/apache/cocoon/
forms/formmodel/Field.java /cocoon/branches/BRANCH_2_1_X/status.xml
Posted by Joerg Heinicke <jo...@gmx.de>.
On 28.05.2005 03:20, Antonio Gallardo wrote:
> Hi Joerg:
>
> I am glad to see you active on the list again. As I told you in GT2004, I
> like your radar over my work. :-)
Hi Antonio,
yeah, I'm back again at least for a few days (starting last weekend,
ending tomorrow). I hope to get my internet access in the next days for
being permantly online again.
> I only got one reply, I didn't knew if the community was in favor of the
> change (I assumed a lazy approval).
I have not been reading your RT, I'm somewhat behind my mails (mid of
february for dev and beginning of march for users list). I only saw your
commit mail (there I'm up-to-date ;-) ).
> This code was done mostly as a proof
> of concept. To see if this really will improve the performance.
>
> After all, I think there is a good news: The proof was successful. While
> Using 3 combos inside a repeater I can see the code runs 2 to 3 times
> faster than before. When the repeater grows, there is almost no
> performance penality now.
>
> Now, I am also thinking to improve now other pieces, for example, there is
> another big time penality inside selection-lists. Again when we are inside
> a repeaters. When we change the value of a selection-list using something
> like that:
>
> <fd:on-value-changed>
> <fd:javascript>
> myWidget.setSelectionList("cocoon:/mySelectionList.data?id=" + value);
> </fd:javascript>
> </fd:on-value-changed>
>
> Here again, the current behavior is to reread the data. And I noted in
> some cases is posible the same combo will be there over and over so a
> cache improvement will save a lot of time again. But this is another
> story. First, I will try to fix the current one.
I understood your goals. Maybe the selection list can be configured to
be buffered for the request. The SelectionListHandler is then not
instantiated for each generateSAXFragment()-call, but only once, stores
the events in SAXBuffer and replays them for all selection lists of that
type for one request.
Now I'm going to read your initial RT and Sylvain's answer :-)
Joerg