You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cocoon.apache.org by Vadim Gritsenko <va...@reverycodes.com> on 2005/03/08 23:48:15 UTC
Re: svn commit: r156143 - cocoon/trunk/src/java/org/apache/cocoon/generation/JXTemplateGenerator.java
cocoon/trunk/status.xml
lgawron@apache.org wrote:
> Author: lgawron
> Date: Fri Mar 4 00:39:53 2005
> New Revision: 156143
>
> URL: http://svn.apache.org/viewcvs?view=rev&rev=156143
> Log:
> Fix thread safety problem in JXTemplateGenerator.setup() concerning template script reparsing.
>
> Modified: cocoon/trunk/src/java/org/apache/cocoon/generation/JXTemplateGenerator.java
> URL: http://svn.apache.org/viewcvs/cocoon/trunk/src/java/org/apache/cocoon/generation/JXTemplateGenerator.java?view=diff&r1=156142&r2=156143
> ==============================================================================
> --- cocoon/trunk/src/java/org/apache/cocoon/generation/JXTemplateGenerator.java (original)
> +++ cocoon/trunk/src/java/org/apache/cocoon/generation/JXTemplateGenerator.java Fri Mar 4 00:39:53 2005
> @@ -2357,7 +2357,6 @@
> valid = startEvent.compileTime.isValid(validity);
> }
> if (valid != SourceValidity.VALID) {
> - cache.remove(uri);
> regenerate = true;
> }
> } else {
What good this does? Second thread, instead of quick fail on first test (if
(startEvent != null)), will go through complete validity check. And it will
arrive to the same result, that it need to re-generate from source. So, it will
arrive to the same SourceUtil.parse line.
PS I'd mark the bug INVALID as I do not see what's broken. Yes, it can parse it
twice. It's not an error by itself. Wrapping whole parsing block into the
synchronized() is abviously not a solution too, especially for larger sites.
Vadim
Re: svn commit: r156143 - cocoon/trunk/src/java/org/apache/cocoon/generation/JXTemplateGenerator.java
cocoon/trunk/status.xml
Posted by Leszek Gawron <lg...@apache.org>.
Vadim Gritsenko wrote:
> Leszek Gawron wrote:
>
>> OT I wonder why we never persisted XSPs though.
>
>
> We did. XSP source files, class files are cached in the file system.
> When loaded, ComponentSelector is created for each XSP, which pools
> instances.
Still as far as I remember restarting container caused all XSPs to
regenerate.
--
Leszek Gawron MobileBox
lgawron@apache.org http://www.mobilebox.pl
Re: svn commit: r156143 - cocoon/trunk/src/java/org/apache/cocoon/generation/JXTemplateGenerator.java
cocoon/trunk/status.xml
Posted by Vadim Gritsenko <va...@reverycodes.com>.
Leszek Gawron wrote:
> OT I wonder why we never persisted
> XSPs though.
We did. XSP source files, class files are cached in the file system. When
loaded, ComponentSelector is created for each XSP, which pools instances.
Vadim
Re: svn commit: r156143 - cocoon/trunk/src/java/org/apache/cocoon/generation/JXTemplateGenerator.java
cocoon/trunk/status.xml
Posted by Leszek Gawron <lg...@apache.org>.
Vadim Gritsenko wrote:
> Leszek Gawron wrote:
>
>> Sorry for such a long delay. I had been busy lately.
>>
>> Vadim Gritsenko wrote:
>>
>>> Also, private static cache map should go in favor of the store
>>> component. Otherwise, larger site might simply run out of memory
>>> 'cause JXTemplateGenerator ate all of it.
>>
>>
>>
>> I'm not into store component usage. Could you give me some directions?
>>
>> What should I take into consideration when building a key for storing
>> a compiled script?
>
>
> Key should be unique. So if you prefix it with something like "jxtg:"
> and add source URI of the template, and source key, it should be good
> enough.
>
> Reason for the source key - we want to cache all variations of the
> template even if they are created from the same source URI.
Yep, I got that.
>> I know there are different stores in cocoon (transient/persistent).
>> Which implementation can we use?
>
>
> Is it Serializable?
not now.
> Does it make sense to preserve cached templates
> across sever restarts?
not sure
> Does it take much time to re-parse template if it
> is lost from the store?
The reparsing speed is just fine. OT I wonder why we never persisted
XSPs though.
Let's go with transient store first.
--
Leszek Gawron MobileBox
lgawron@apache.org http://www.mobilebox.pl
Re: svn commit: r156143 - cocoon/trunk/src/java/org/apache/cocoon/generation/JXTemplateGenerator.java
cocoon/trunk/status.xml
Posted by Vadim Gritsenko <va...@reverycodes.com>.
Leszek Gawron wrote:
> Vadim Gritsenko wrote:
>
>> Leszek Gawron wrote:
>>
>>> Sorry for such a long delay. I had been busy lately.
>>>
>>> Vadim Gritsenko wrote:
>>>
>>>> Also, private static cache map should go in favor of the store
>>>> component. Otherwise, larger site might simply run out of memory
>>>> 'cause JXTemplateGenerator ate all of it.
>>>
>>>
>>> I'm not into store component usage. Could you give me some directions?
>>>
>>> What should I take into consideration when building a key for storing
>>> a compiled script?
>>
>>
>> Key should be unique. So if you prefix it with something like "jxtg:"
>> and add source URI of the template, and source key, it should be good
>> enough.
>>
>> Reason for the source key - we want to cache all variations of the
>> template even if they are created from the same source URI.
>
> I do not get something: while resolving a source from uri how do I get
> access to source key? The source provides only source validity and there
> is no way to calculate it manually.
My mix-up... There is no key in source, only SystemID, so that is all you need.
Vadim
Re: svn commit: r156143 - cocoon/trunk/src/java/org/apache/cocoon/generation/JXTemplateGenerator.java
cocoon/trunk/status.xml
Posted by Leszek Gawron <lg...@apache.org>.
Vadim Gritsenko wrote:
> Leszek Gawron wrote:
>
>> Sorry for such a long delay. I had been busy lately.
>>
>> Vadim Gritsenko wrote:
>>
>>> Also, private static cache map should go in favor of the store
>>> component. Otherwise, larger site might simply run out of memory
>>> 'cause JXTemplateGenerator ate all of it.
>>
>>
>>
>> I'm not into store component usage. Could you give me some directions?
>>
>> What should I take into consideration when building a key for storing
>> a compiled script?
>
>
> Key should be unique. So if you prefix it with something like "jxtg:"
> and add source URI of the template, and source key, it should be good
> enough.
>
> Reason for the source key - we want to cache all variations of the
> template even if they are created from the same source URI.
I do not get something: while resolving a source from uri how do I get
access to source key? The source provides only source validity and there
is no way to calculate it manually.
--
Leszek Gawron MobileBox
lgawron@apache.org http://www.mobilebox.pl
Re: svn commit: r156143 - cocoon/trunk/src/java/org/apache/cocoon/generation/JXTemplateGenerator.java
cocoon/trunk/status.xml
Posted by Vadim Gritsenko <va...@reverycodes.com>.
Leszek Gawron wrote:
> Sorry for such a long delay. I had been busy lately.
>
> Vadim Gritsenko wrote:
>
>> Also, private static cache map should go in favor of the store
>> component. Otherwise, larger site might simply run out of memory
>> 'cause JXTemplateGenerator ate all of it.
>
>
> I'm not into store component usage. Could you give me some directions?
>
> What should I take into consideration when building a key for storing a
> compiled script?
Key should be unique. So if you prefix it with something like "jxtg:" and add
source URI of the template, and source key, it should be good enough.
Reason for the source key - we want to cache all variations of the template even
if they are created from the same source URI.
> I know there are different stores in cocoon (transient/persistent).
> Which implementation can we use?
Is it Serializable? Does it make sense to preserve cached templates across sever
restarts? Does it take much time to re-parse template if it is lost from the
store? If no to any of these, then use transient store.
Vadim
Re: svn commit: r156143 - cocoon/trunk/src/java/org/apache/cocoon/generation/JXTemplateGenerator.java
cocoon/trunk/status.xml
Posted by Leszek Gawron <lg...@apache.org>.
Sorry for such a long delay. I had been busy lately.
Vadim Gritsenko wrote:
> Ok, got it. In this case proper solution is not to use cache in the
> generate, but obtain startEvent instance in the setup and save it for
> generate in member variable. In this case, generate method won't have a
> chance of accessing modified (in this case - removed) data.
>
> Also, private static cache map should go in favor of the store
> component. Otherwise, larger site might simply run out of memory 'cause
> JXTemplateGenerator ate all of it.
I'm not into store component usage. Could you give me some directions?
What should I take into consideration when building a key for storing a
compiled script?
I know there are different stores in cocoon (transient/persistent).
Which implementation can we use?
--
Leszek Gawron MobileBox
lgawron@apache.org http://www.mobilebox.pl
Re: svn commit: r156143 - cocoon/trunk/src/java/org/apache/cocoon/generation/JXTemplateGenerator.java
cocoon/trunk/status.xml
Posted by Vadim Gritsenko <va...@reverycodes.com>.
Leszek Gawron wrote:
> Vadim Gritsenko wrote:
>
>> lgawron@apache.org wrote:
>>
>>> Author: lgawron
>>> Date: Fri Mar 4 00:39:53 2005
>>> New Revision: 156143
>>>
>>> URL: http://svn.apache.org/viewcvs?view=rev&rev=156143
>>> Log:
>>> Fix thread safety problem in JXTemplateGenerator.setup() concerning
>>> template script reparsing.
>>>
>>> Modified:
>>> cocoon/trunk/src/java/org/apache/cocoon/generation/JXTemplateGenerator.java
>>>
>>> URL:
>>> http://svn.apache.org/viewcvs/cocoon/trunk/src/java/org/apache/cocoon/generation/JXTemplateGenerator.java?view=diff&r1=156142&r2=156143
>>>
>>> ==============================================================================
>>>
>>> ---
>>> cocoon/trunk/src/java/org/apache/cocoon/generation/JXTemplateGenerator.java
>>> (original)
>>> +++
>>> cocoon/trunk/src/java/org/apache/cocoon/generation/JXTemplateGenerator.java
>>> Fri Mar 4 00:39:53 2005
>>> @@ -2357,7 +2357,6 @@
>>> valid =
>>> startEvent.compileTime.isValid(validity);
>>> }
>>> if (valid != SourceValidity.VALID) {
>>> - cache.remove(uri);
>>> regenerate = true;
>>> }
>>> } else {
>>
>>
>>
>> What good this does? Second thread, instead of quick fail on first
>> test (if (startEvent != null)), will go through complete validity
>> check. And it will arrive to the same result, that it need to
>> re-generate from source. So, it will arrive to the same
>> SourceUtil.parse line.
>>
>> PS I'd mark the bug INVALID as I do not see what's broken. Yes, it can
>> parse it twice. It's not an error by itself. Wrapping whole parsing
>> block into the synchronized() is abviously not a solution too,
>> especially for larger sites.
>
> Maybe I had it wrong but imagine the situation with two threads:
> Thread1 Thread2
> 1) JXTG.setup()
> got script,
> script is valid
> JXTG.setup() finish
>
> 2) JXTG.setup()
> script invalid so removed from cache
>
> 3) JXTG.generate()
> script = cache.get(...)
> script is null as second
> thread removed it while
> it should still be there
> because it was valid at the
> setup() stage
>
> 4) script reparsed
> JXTG.setup() finish()
>
>
> This is not a race condition between two JXTG.setup() but with
> JXTG.generate() in first thread and JXTG.setup() in second one.
Ok, got it. In this case proper solution is not to use cache in the generate,
but obtain startEvent instance in the setup and save it for generate in member
variable. In this case, generate method won't have a chance of accessing
modified (in this case - removed) data.
Also, private static cache map should go in favor of the store component.
Otherwise, larger site might simply run out of memory 'cause JXTemplateGenerator
ate all of it.
Vadim
Re: svn commit: r156143 - cocoon/trunk/src/java/org/apache/cocoon/generation/JXTemplateGenerator.java
cocoon/trunk/status.xml
Posted by Leszek Gawron <lg...@mobilebox.pl>.
Vadim Gritsenko wrote:
> lgawron@apache.org wrote:
>
>> Author: lgawron
>> Date: Fri Mar 4 00:39:53 2005
>> New Revision: 156143
>>
>> URL: http://svn.apache.org/viewcvs?view=rev&rev=156143
>> Log:
>> Fix thread safety problem in JXTemplateGenerator.setup() concerning
>> template script reparsing.
>>
>> Modified:
>> cocoon/trunk/src/java/org/apache/cocoon/generation/JXTemplateGenerator.java
>>
>> URL:
>> http://svn.apache.org/viewcvs/cocoon/trunk/src/java/org/apache/cocoon/generation/JXTemplateGenerator.java?view=diff&r1=156142&r2=156143
>>
>> ==============================================================================
>>
>> ---
>> cocoon/trunk/src/java/org/apache/cocoon/generation/JXTemplateGenerator.java
>> (original)
>> +++
>> cocoon/trunk/src/java/org/apache/cocoon/generation/JXTemplateGenerator.java
>> Fri Mar 4 00:39:53 2005
>> @@ -2357,7 +2357,6 @@
>> valid =
>> startEvent.compileTime.isValid(validity);
>> }
>> if (valid != SourceValidity.VALID) {
>> - cache.remove(uri);
>> regenerate = true;
>> }
>> } else {
>
>
> What good this does? Second thread, instead of quick fail on first test
> (if (startEvent != null)), will go through complete validity check. And
> it will arrive to the same result, that it need to re-generate from
> source. So, it will arrive to the same SourceUtil.parse line.
>
> PS I'd mark the bug INVALID as I do not see what's broken. Yes, it can
> parse it twice. It's not an error by itself. Wrapping whole parsing
> block into the synchronized() is abviously not a solution too,
> especially for larger sites.
Maybe I had it wrong but imagine the situation with two threads:
Thread1 Thread2
1) JXTG.setup()
got script,
script is valid
JXTG.setup() finish
2) JXTG.setup()
script invalid so removed from cache
3) JXTG.generate()
script = cache.get(...)
script is null as second
thread removed it while
it should still be there
because it was valid at the
setup() stage
4) script reparsed
JXTG.setup() finish()
This is not a race condition between two JXTG.setup() but with
JXTG.generate() in first thread and JXTG.setup() in second one.
--
Leszek Gawron lgawron@mobilebox.pl
IT Manager MobileBox sp. z o.o.
+48 (61) 855 06 67 http://www.mobilebox.pl
mobile: +48 (501) 720 812 fax: +48 (61) 853 29 65