You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tapestry.apache.org by Denis Stepanov <de...@gmail.com> on 2011/07/19 11:53:08 UTC
MarkupWriterImpl memory improvement
Hi all,
I had noticed that in MarkupWriterImpl every time when element event start/end is fired, collection of listeners is recreated, probably to prevent CME, better would be recreate it only when it is modified, see the issue with a patch: https://issues.apache.org/jira/browse/TAP5-1532.
Denis
Re: MarkupWriterImpl memory improvement
Posted by Denis Stepanov <de...@gmail.com>.
It seems that Howard fixed it by TAP5-1581.
Denis
On Jul 19, 2011, at 4:14 PM, Denis Stepanov wrote:
> Hi Igor,
>
> 1) Some devs are using git, so I thought it's ok.
> 2) You're right! It should be assigned after the change.
>
> Denis
>
> On Jul 19, 2011, at 3:02 PM, Igor Drobiazko wrote:
>
>> Hi Denis,
>>
>> I have some concerns about your patch.
>>
>> 1) The Apache's official source control system is Subversion. I'm not sure
>> if git patches may be applied to SVN. Your patch will be more likely applied
>> if you provide it SVN's diff.
>>
>> 2) I think your patch can lead to CME. Imagine that fireElementDidStart()
>> starts iterating exactly when the invocation of removeListener() method is
>> currently between lines:
>>
>> listeners = CollectionFactory.newList(listeners);
>>
>> and
>>
>> listeners.remove(listener);
>>
>> Regards
>>
>> On Tue, Jul 19, 2011 at 11:53 AM, Denis Stepanov
>> <de...@gmail.com>wrote:
>>
>>> Hi all,
>>>
>>> I had noticed that in MarkupWriterImpl every time when element event
>>> start/end is fired, collection of listeners is recreated, probably to
>>> prevent CME, better would be recreate it only when it is modified, see the
>>> issue with a patch: https://issues.apache.org/jira/browse/TAP5-1532.
>>>
>>> Denis
>>
>>
>>
>>
>> --
>> Best regards,
>>
>> Igor Drobiazko
>> http://tapestry5.de
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tapestry.apache.org
For additional commands, e-mail: dev-help@tapestry.apache.org
Re: MarkupWriterImpl memory improvement
Posted by Denis Stepanov <de...@gmail.com>.
Hi Igor,
1) Some devs are using git, so I thought it's ok.
2) You're right! It should be assigned after the change.
Denis
On Jul 19, 2011, at 3:02 PM, Igor Drobiazko wrote:
> Hi Denis,
>
> I have some concerns about your patch.
>
> 1) The Apache's official source control system is Subversion. I'm not sure
> if git patches may be applied to SVN. Your patch will be more likely applied
> if you provide it SVN's diff.
>
> 2) I think your patch can lead to CME. Imagine that fireElementDidStart()
> starts iterating exactly when the invocation of removeListener() method is
> currently between lines:
>
> listeners = CollectionFactory.newList(listeners);
>
> and
>
> listeners.remove(listener);
>
> Regards
>
> On Tue, Jul 19, 2011 at 11:53 AM, Denis Stepanov
> <de...@gmail.com>wrote:
>
>> Hi all,
>>
>> I had noticed that in MarkupWriterImpl every time when element event
>> start/end is fired, collection of listeners is recreated, probably to
>> prevent CME, better would be recreate it only when it is modified, see the
>> issue with a patch: https://issues.apache.org/jira/browse/TAP5-1532.
>>
>> Denis
>
>
>
>
> --
> Best regards,
>
> Igor Drobiazko
> http://tapestry5.de
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tapestry.apache.org
For additional commands, e-mail: dev-help@tapestry.apache.org
Re: MarkupWriterImpl memory improvement
Posted by David Rees <dr...@gmail.com>.
On Wed, Jul 20, 2011 at 11:01 PM, Kalle Korhonen
<ka...@gmail.com> wrote:
> On Wed, Jul 20, 2011 at 10:44 PM, David Rees <dr...@gmail.com> wrote:
>> On Tue, Jul 19, 2011 at 6:02 AM, Igor Drobiazko
>> A patch is a patch. It doesn't matter if it was generated using SVN
>> or git or your magical unicorn - the patch will apply regardless.
>
> Hmm.. perhaps try the tools first in practice and then comment on
> whether it matters or not. See for example
> http://stackoverflow.com/questions/708202/git-format-patch-to-be-svn-compatible
> and discussion on 1.7 subversion.
What am I missing?
$ svn checkout https://svn.apache.org/repos/asf/tapestry/tapestry5/trunk/
tapestry-project
$ cd tapestry-project
$ wget "https://issues.apache.org/jira/secure/attachment/12480534/0001-MarkupWriterImpl-improvement.patch"
$ patch -p1 < 0001-MarkupWriterImpl-improvement.patch
Works fine for me...
-Dave
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tapestry.apache.org
For additional commands, e-mail: dev-help@tapestry.apache.org
Re: MarkupWriterImpl memory improvement
Posted by Kalle Korhonen <ka...@gmail.com>.
On Wed, Jul 20, 2011 at 10:44 PM, David Rees <dr...@gmail.com> wrote:
> On Tue, Jul 19, 2011 at 6:02 AM, Igor Drobiazko
> A patch is a patch. It doesn't matter if it was generated using SVN
> or git or your magical unicorn - the patch will apply regardless.
Hmm.. perhaps try the tools first in practice and then comment on
whether it matters or not. See for example
http://stackoverflow.com/questions/708202/git-format-patch-to-be-svn-compatible
and discussion on 1.7 subversion.
> git's main benefit is if you host a public tree with the changes -
> then others can pull in those changes with a simple command - rather
> than going through the trouble of applying a patch.
I doubt you'll find many git users who'd agree that's the main benefit of git.
Kalle
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tapestry.apache.org
For additional commands, e-mail: dev-help@tapestry.apache.org
Re: MarkupWriterImpl memory improvement
Posted by David Rees <dr...@gmail.com>.
On Tue, Jul 19, 2011 at 6:02 AM, Igor Drobiazko
<ig...@gmail.com> wrote:
> I have some concerns about your patch.
>
> 1) The Apache's official source control system is Subversion. I'm not sure
> if git patches may be applied to SVN. Your patch will be more likely applied
> if you provide it SVN's diff.
A patch is a patch. It doesn't matter if it was generated using SVN
or git or your magical unicorn - the patch will apply regardless.
git's main benefit is if you host a public tree with the changes -
then others can pull in those changes with a simple command - rather
than going through the trouble of applying a patch.
-Dave
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tapestry.apache.org
For additional commands, e-mail: dev-help@tapestry.apache.org
Re: MarkupWriterImpl memory improvement
Posted by Igor Drobiazko <ig...@gmail.com>.
I'm not aware of anything official but I guess the most of the active
committers are still using plain svn. That's why I think it's more likely
for a patch to be applied if it is in svn format.
On Wed, Jul 20, 2011 at 8:08 PM, Kalle Korhonen
<ka...@gmail.com>wrote:
> On Tue, Jul 19, 2011 at 6:02 AM, Igor Drobiazko
> <ig...@gmail.com> wrote:
> > 1) The Apache's official source control system is Subversion. I'm not
> sure
> > if git patches may be applied to SVN. Your patch will be more likely
> applied
> > if you provide it SVN's diff.
>
> Igor, others - is there anything official anywhere whether patches
> should be in SVN format? Seems to me it'll be easier for committers as
> well to work on git issue branches and apply git patches than svn
> ones. I'm fine with either, but just asking for clarification.
>
> Kalle
>
>
> > On Tue, Jul 19, 2011 at 11:53 AM, Denis Stepanov
> > <de...@gmail.com>wrote:
> >
> >> Hi all,
> >>
> >> I had noticed that in MarkupWriterImpl every time when element event
> >> start/end is fired, collection of listeners is recreated, probably to
> >> prevent CME, better would be recreate it only when it is modified, see
> the
> >> issue with a patch: https://issues.apache.org/jira/browse/TAP5-1532.
> >>
> >> Denis
> >
> >
> >
> >
> > --
> > Best regards,
> >
> > Igor Drobiazko
> > http://tapestry5.de
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tapestry.apache.org
> For additional commands, e-mail: dev-help@tapestry.apache.org
>
>
--
Best regards,
Igor Drobiazko
http://tapestry5.de
Re: MarkupWriterImpl memory improvement
Posted by Howard Lewis Ship <hl...@gmail.com>.
I'm probably good with either, though it would be helpful if the patch
submission identified which type it was.
On Wed, Jul 20, 2011 at 11:08 AM, Kalle Korhonen
<ka...@gmail.com> wrote:
> On Tue, Jul 19, 2011 at 6:02 AM, Igor Drobiazko
> <ig...@gmail.com> wrote:
>> 1) The Apache's official source control system is Subversion. I'm not sure
>> if git patches may be applied to SVN. Your patch will be more likely applied
>> if you provide it SVN's diff.
>
> Igor, others - is there anything official anywhere whether patches
> should be in SVN format? Seems to me it'll be easier for committers as
> well to work on git issue branches and apply git patches than svn
> ones. I'm fine with either, but just asking for clarification.
>
> Kalle
>
>
>> On Tue, Jul 19, 2011 at 11:53 AM, Denis Stepanov
>> <de...@gmail.com>wrote:
>>
>>> Hi all,
>>>
>>> I had noticed that in MarkupWriterImpl every time when element event
>>> start/end is fired, collection of listeners is recreated, probably to
>>> prevent CME, better would be recreate it only when it is modified, see the
>>> issue with a patch: https://issues.apache.org/jira/browse/TAP5-1532.
>>>
>>> Denis
>>
>>
>>
>>
>> --
>> Best regards,
>>
>> Igor Drobiazko
>> http://tapestry5.de
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tapestry.apache.org
> For additional commands, e-mail: dev-help@tapestry.apache.org
>
>
--
Howard M. Lewis Ship
Creator of Apache Tapestry
The source for Tapestry training, mentoring and support. Contact me to
learn how I can get you up and productive in Tapestry fast!
(971) 678-5210
http://howardlewisship.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tapestry.apache.org
For additional commands, e-mail: dev-help@tapestry.apache.org
Re: MarkupWriterImpl memory improvement
Posted by Kalle Korhonen <ka...@gmail.com>.
On Tue, Jul 19, 2011 at 6:02 AM, Igor Drobiazko
<ig...@gmail.com> wrote:
> 1) The Apache's official source control system is Subversion. I'm not sure
> if git patches may be applied to SVN. Your patch will be more likely applied
> if you provide it SVN's diff.
Igor, others - is there anything official anywhere whether patches
should be in SVN format? Seems to me it'll be easier for committers as
well to work on git issue branches and apply git patches than svn
ones. I'm fine with either, but just asking for clarification.
Kalle
> On Tue, Jul 19, 2011 at 11:53 AM, Denis Stepanov
> <de...@gmail.com>wrote:
>
>> Hi all,
>>
>> I had noticed that in MarkupWriterImpl every time when element event
>> start/end is fired, collection of listeners is recreated, probably to
>> prevent CME, better would be recreate it only when it is modified, see the
>> issue with a patch: https://issues.apache.org/jira/browse/TAP5-1532.
>>
>> Denis
>
>
>
>
> --
> Best regards,
>
> Igor Drobiazko
> http://tapestry5.de
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tapestry.apache.org
For additional commands, e-mail: dev-help@tapestry.apache.org
Re: MarkupWriterImpl memory improvement
Posted by Igor Drobiazko <ig...@gmail.com>.
Hi Denis,
I have some concerns about your patch.
1) The Apache's official source control system is Subversion. I'm not sure
if git patches may be applied to SVN. Your patch will be more likely applied
if you provide it SVN's diff.
2) I think your patch can lead to CME. Imagine that fireElementDidStart()
starts iterating exactly when the invocation of removeListener() method is
currently between lines:
listeners = CollectionFactory.newList(listeners);
and
listeners.remove(listener);
Regards
On Tue, Jul 19, 2011 at 11:53 AM, Denis Stepanov
<de...@gmail.com>wrote:
> Hi all,
>
> I had noticed that in MarkupWriterImpl every time when element event
> start/end is fired, collection of listeners is recreated, probably to
> prevent CME, better would be recreate it only when it is modified, see the
> issue with a patch: https://issues.apache.org/jira/browse/TAP5-1532.
>
> Denis
--
Best regards,
Igor Drobiazko
http://tapestry5.de