You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@excalibur.apache.org by Giacomo Pati <gi...@apache.org> on 2004/11/09 21:20:17 UTC
refactoring StoreJanitorImpl
Hy guys
We had the problem over in Cocoon that the StoreJanitoImpl class cannot
be easily subclassed as we have replaces the deprecated Excalibur event
package
with our own we'd like the
StoreJanitorImpl to be used the same way wrt thread usage. So
we'd like to be able to overwrite the start and run method to have our
approach to launch regularly background tasks but we want the logic to
be the same as in the subclass (seams to be sofisticated, no need to
change or duplicate that).
So my refactoring is that all the logic in the run method is refactored
out in its own protected method.
WDYT
As I've propably lost my committer right since the move from Avalon to
Excalibur how about a patch (well, couldn't see a place neither in Jira
nor Bugzilla to place that patch)?
--
Giacomo Pati
Otego AG, Switzerland - http://www.otego.com
Orixo, the XML business alliance - http://www.orixo.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@excalibur.apache.org
For additional commands, e-mail: dev-help@excalibur.apache.org
Apache Excalibur Project -- URL: http://excalibur.apache.org/
Re: refactoring StoreJanitorImpl
Posted by Berin Loritsch <bl...@d-haven.org>.
Giacomo Pati wrote:
>
> Vadim brought up the idea to contribute the Cocoon Event package
> replacement (RunnableManager) to Excalibur. Personally, I'm not sure
> about it as in the end we have the Event package back here in a
> different form. So, maybe you guys have a look at it at
> http://svn.apache.org/repos/asf/cocoon/trunk/src/java/org/apache/cocoon/components/thread/
> and give us your thoughts about it.
I can take a look at it...
>
> Honestly, I didn't find the rason why the Event package has been
> deprecated at all (and I just took it as a community decission I've not
> followed)
Mainly because there is a much better tested version of it over at
http://dist.d-haven.org/d-haven-event/
http://api.d-haven.org/event/
http://scm.d-haven.org/svn/d-haven/trunk/event/
which is pretty robust, and the latest version of it has fixed some
long time assumptions that the old Event package lived under.
Since I am about the only one really maintaining the Event package,
I reworked it so that it is compatible with Java5, but implemented
with Java 1.4 (I had to remove the name conflict for Queue). I also
reimplemented a lot of the Command package stuff so that it really
did what it was advertising.
In the end, it just seemed better to use that rather than maintaining
a lesser version here that barely had any tests.
The main reason I started the fork of Event was to teach myself TDD.
In the process I discovered a few things wrong with the old code,
and had to make some incompatible changes to fix them. Since what I
had turned out so well, I released it.
--
"Programming today is a race between software engineers striving to
build bigger and better idiot-proof programs, and the Universe trying to
produce bigger and better idiots. So far, the Universe is winning."
- Rich Cook
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@excalibur.apache.org
For additional commands, e-mail: dev-help@excalibur.apache.org
Apache Excalibur Project -- URL: http://excalibur.apache.org/
Re: refactoring StoreJanitorImpl
Posted by Berin Loritsch <bl...@d-haven.org>.
Leo Simons wrote:
> Giacomo Pati wrote:
>
>>> so all cocoon committers can commit to all of excalibur. Cocoon is
>>> one of the most important users of the excalibur codebase, and we
>>> like to keep the barrier as low as possible (trusting of course
>>> no-one makes a mess out of things :-D).
>>
>>
>> Vadim brought up the idea to contribute the Cocoon Event package
>> replacement (RunnableManager) to Excalibur. Personally, I'm not sure
>> about it as in the end we have the Event package back here in a
>> different form. So, maybe you guys have a look at it at
>> http://svn.apache.org/repos/asf/cocoon/trunk/src/java/org/apache/cocoon/components/thread/
>> and give us your thoughts about it.
>
>
> Hmm. I don't know that much about the event stuff, primarily seen it
> from a user perspective as it was used in fortress.
>
> My general experience in the past when codebases moved from cocoon to
> avalon was that it didn't do those codebases much good, so I'm not so
> sure this'll benefit anyone....
>
> Berin, care to comment?
>
Event was originally a fork/port of the SEDA stuff in a sane API.
It was ported with the intention of building a SEDA framework on top
of Avalon. Assumptions were made about the threading models and
fixes were done by trial and error. There are a couple classes
in there that will conflict with Java5, but as that wasn't anywhere
around at the time I created it I had no way of knowing.
--
"Programming today is a race between software engineers striving to
build bigger and better idiot-proof programs, and the Universe trying to
produce bigger and better idiots. So far, the Universe is winning."
- Rich Cook
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@excalibur.apache.org
For additional commands, e-mail: dev-help@excalibur.apache.org
Apache Excalibur Project -- URL: http://excalibur.apache.org/
Re: refactoring StoreJanitorImpl
Posted by Leo Simons <ls...@jicarilla.org>.
Giacomo Pati wrote:
>> so all cocoon committers can commit to all of excalibur. Cocoon is one
>> of the most important users of the excalibur codebase, and we like to
>> keep the barrier as low as possible (trusting of course no-one makes a
>> mess out of things :-D).
>
> Vadim brought up the idea to contribute the Cocoon Event package
> replacement (RunnableManager) to Excalibur. Personally, I'm not sure
> about it as in the end we have the Event package back here in a
> different form. So, maybe you guys have a look at it at
> http://svn.apache.org/repos/asf/cocoon/trunk/src/java/org/apache/cocoon/components/thread/
> and give us your thoughts about it.
Hmm. I don't know that much about the event stuff, primarily seen it
from a user perspective as it was used in fortress.
My general experience in the past when codebases moved from cocoon to
avalon was that it didn't do those codebases much good, so I'm not so
sure this'll benefit anyone....
Berin, care to comment?
> Honestly, I didn't find the rason why the Event package has been
> deprecated at all (and I just took it as a community decission I've not
> followed)
as I recall...
Over at avalon, at some point it was decided to start maintaining the
event packages over at d-haven.org. Since Berin started working on it
over there and stopped working on it at avalon (many people stopped
working on many things at avalon...), it was also decided to port the
excalibur codebases (in particular, fortress) over to the d-haven event
packages (which are better than the old ones in absolutely every way).
With all that settled and no-one left here maintaining our own event
package (but Berin as the primary maintainer of course being close by
and involved) it only made sense to deprecate it in favor of the d-haven
package.
I'm a bit surprised this didn't pop up on the cocoon-dev list earlier, I
vaguely recall asking about what the impact would be on cocoon. And that
raising no concern. Mind you, this was probably all in a period (o, how
vague te memory can be...) where just about the entire cocoon crew had
just decided that reducing dependency on avalon as much as possible was
a Good Thing, so that could be a factor as well. I dunno.
Hmm.
I think the first thing to make sure is that cocoon is confident it can
trust excalibur as a dependency for the existing stuff, before thinking
about increasing that dependency again.
Does that make any sense?
cheers,
Leo
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@excalibur.apache.org
For additional commands, e-mail: dev-help@excalibur.apache.org
Apache Excalibur Project -- URL: http://excalibur.apache.org/
Re: refactoring StoreJanitorImpl
Posted by Berin Loritsch <bl...@d-haven.org>.
On Wed, 2004-11-10 at 15:49, Giacomo Pati wrote:
> On Wed, 10 Nov 2004, Berin Loritsch wrote:
>
> > Giacomo Pati wrote:
> >
> >> Vadim brought up the idea to contribute the Cocoon Event package
> >> replacement (RunnableManager) to Excalibur. Personally, I'm not sure
> >> about it as in the end we have the Event package back here in a different
> >> form. So, maybe you guys have a look at it at
> >> http://svn.apache.org/repos/asf/cocoon/trunk/src/java/org/apache/cocoon/components/thread/
> >> and give us your thoughts about it.
> >
> > OK, here is my surmizing of what is in that package:
> >
> > 1) It is a thin wrapper around the Doug Lea Concurrent library
>
> Yup.
>
> > 2) That makes it pretty close to Java5 java.util.concurrent stuff
> > 3) The LinkedQueue will has race conditions waiting to happen with the
> > size tracking--only use it with one thread at a time.
>
> Hmm, you mean the insert method, right?
>
> > 4) The RunnableManager is a lot of code without one test--it makes me
> > nervous.
>
> Sure. Tests will follow.
>
> > For the most part it is a big departure from the Excalibur/D-Haven event
> > packages and it is pretty close to the Doug Lea Concurrent library. I
> > can see some potential issues with synchronization--especially as it
> > relates to tracking queue size.
>
> Where?
Take a look at the LinkedListQueue.
There is no synchronization around the queue size, and that is altered
prior to calling the underlying channel. If multiple threads access
that at the same time it will get out of sync. It can also potentially
get out of sync when the underlying channel generates an error and
doesn't perform the expected operation. The size isn't even declared
volatile (which is of dubious value as it is).
>
> > The RunnableManager is a non-trivial class that might actually be
> > better split up.
>
> Any suggestions are welcome.
I don't have anything specific, but as I started writing the tests
for the DefaultCommandManager in D-Haven event, it became increasingly
clear that I needed to split it up and test things separately. A big
clue is if you have any inner classes, it might be beneficial to
split them out to first class citizens. That's where I started when
I did that work.
>
> > I highly recommend writing a bunch of test cases
> > with JUnit and Clover until the whole thing is covered by tests.
> > While I know Vadim does good work, it surprised me the number of
> > issues I found in my own code.
>
> Don't blame Vadim about it, the RunnableManager is my s**t :-). Many
> thank for looking at it. I'd rather stick with the d-haven event package
> but the cocoon community has decided otherwise (you either know or can
> query theaimsgroup about it).
Sorry, I saw Vadim's name on some of the classes, and assumed it was
on all of them.
Did they look at the D-Haven Event package? Another reason for me
splitting it out of Avalon at the time was... well... due to
interpersonal volatility at the time.
I know that Stefano would really like to have as much in house for
Cocoon so that it doesn't look like building on sand any more, but
some libraries are just not easy--to write or test.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@excalibur.apache.org
For additional commands, e-mail: dev-help@excalibur.apache.org
Apache Excalibur Project -- URL: http://excalibur.apache.org/
Re: refactoring StoreJanitorImpl
Posted by Giacomo Pati <gi...@apache.org>.
On Wed, 10 Nov 2004, Berin Loritsch wrote:
> Giacomo Pati wrote:
>
>> Vadim brought up the idea to contribute the Cocoon Event package
>> replacement (RunnableManager) to Excalibur. Personally, I'm not sure
>> about it as in the end we have the Event package back here in a different
>> form. So, maybe you guys have a look at it at
>> http://svn.apache.org/repos/asf/cocoon/trunk/src/java/org/apache/cocoon/components/thread/
>> and give us your thoughts about it.
>
> OK, here is my surmizing of what is in that package:
>
> 1) It is a thin wrapper around the Doug Lea Concurrent library
Yup.
> 2) That makes it pretty close to Java5 java.util.concurrent stuff
> 3) The LinkedQueue will has race conditions waiting to happen with the
> size tracking--only use it with one thread at a time.
Hmm, you mean the insert method, right?
> 4) The RunnableManager is a lot of code without one test--it makes me
> nervous.
Sure. Tests will follow.
> For the most part it is a big departure from the Excalibur/D-Haven event
> packages and it is pretty close to the Doug Lea Concurrent library. I
> can see some potential issues with synchronization--especially as it
> relates to tracking queue size.
Where?
> The RunnableManager is a non-trivial class that might actually be
> better split up.
Any suggestions are welcome.
> I highly recommend writing a bunch of test cases
> with JUnit and Clover until the whole thing is covered by tests.
> While I know Vadim does good work, it surprised me the number of
> issues I found in my own code.
Don't blame Vadim about it, the RunnableManager is my s**t :-). Many
thank for looking at it. I'd rather stick with the d-haven event package
but the cocoon community has decided otherwise (you either know or can
query theaimsgroup about it).
--
Giacomo Pati
Otego AG, Switzerland - http://www.otego.com
Orixo, the XML business alliance - http://www.orixo.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@excalibur.apache.org
For additional commands, e-mail: dev-help@excalibur.apache.org
Apache Excalibur Project -- URL: http://excalibur.apache.org/
Re: refactoring StoreJanitorImpl
Posted by Berin Loritsch <bl...@d-haven.org>.
Giacomo Pati wrote:
> Vadim brought up the idea to contribute the Cocoon Event package
> replacement (RunnableManager) to Excalibur. Personally, I'm not sure
> about it as in the end we have the Event package back here in a
> different form. So, maybe you guys have a look at it at
> http://svn.apache.org/repos/asf/cocoon/trunk/src/java/org/apache/cocoon/components/thread/
> and give us your thoughts about it.
OK, here is my surmizing of what is in that package:
1) It is a thin wrapper around the Doug Lea Concurrent library
2) That makes it pretty close to Java5 java.util.concurrent stuff
3) The LinkedQueue will has race conditions waiting to happen with the
size tracking--only use it with one thread at a time.
4) The RunnableManager is a lot of code without one test--it makes me
nervous.
For the most part it is a big departure from the Excalibur/D-Haven event
packages and it is pretty close to the Doug Lea Concurrent library. I
can see some potential issues with synchronization--especially as it
relates to tracking queue size.
The RunnableManager is a non-trivial class that might actually be
better split up. I highly recommend writing a bunch of test cases
with JUnit and Clover until the whole thing is covered by tests.
While I know Vadim does good work, it surprised me the number of
issues I found in my own code.
--
"Programming today is a race between software engineers striving to
build bigger and better idiot-proof programs, and the Universe trying to
produce bigger and better idiots. So far, the Universe is winning."
- Rich Cook
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@excalibur.apache.org
For additional commands, e-mail: dev-help@excalibur.apache.org
Apache Excalibur Project -- URL: http://excalibur.apache.org/
Re: refactoring StoreJanitorImpl
Posted by Giacomo Pati <gi...@apache.org>.
On Tue, 9 Nov 2004, Leo Simons wrote:
> Giacomo Pati wrote:
>> Hy guys
>
> Hi Giacomo,
>
> good to see you around here :-D
>
>> We had the problem over in Cocoon that the StoreJanitoImpl class cannot
>> be easily subclassed as we have replaces the deprecated Excalibur event
>> package with our own we'd like the StoreJanitorImpl to be used the same
>> way wrt thread usage. So we'd like to be able to overwrite the start and
>> run method to have our approach to launch regularly background tasks but
>> we want the logic to be the same as in the subclass (seams to be
>> sofisticated, no need to change or duplicate that).
>>
>> So my refactoring is that all the logic in the run method is refactored
>> out in its own protected method.
>>
>> WDYT
>
> sure, why not? Go right ahead!
Ok.
>> As I've propably lost my committer right since the move from Avalon to
>> Excalibur how about a patch (well, couldn't see a place neither in Jira
>> nor Bugzilla to place that patch)?
>
> Nah, you've still got that access! SVN has this configured:
>
> [/excalibur]
> @excalibur = rw
> @james = rw
> @cocoon = rw
Sorry, missed that one. Was just greping for my name.
> so all cocoon committers can commit to all of excalibur. Cocoon is one of the
> most important users of the excalibur codebase, and we like to keep the
> barrier as low as possible (trusting of course no-one makes a mess out of
> things :-D).
Vadim brought up the idea to contribute the Cocoon Event package
replacement (RunnableManager) to Excalibur. Personally, I'm not sure
about it as in the end we have the Event package back here in a
different form. So, maybe you guys have a look at it at
http://svn.apache.org/repos/asf/cocoon/trunk/src/java/org/apache/cocoon/components/thread/
and give us your thoughts about it.
Honestly, I didn't find the rason why the Event package has been
deprecated at all (and I just took it as a community decission I've not
followed)
> As an aside, jira does have excalibur and fortress in there:
>
> http://excalibur.apache.org/issue-tracking.html
Ok, thanks, I'll have a look at it, too.
--
Giacomo Pati
Otego AG, Switzerland - http://www.otego.com
Orixo, the XML business alliance - http://www.orixo.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@excalibur.apache.org
For additional commands, e-mail: dev-help@excalibur.apache.org
Apache Excalibur Project -- URL: http://excalibur.apache.org/
Re: refactoring StoreJanitorImpl
Posted by Antonio Gallardo <ag...@agssa.net>.
Leo Simons dijo:
> Nah, you've still got that access! SVN has this configured:
>
> [/excalibur]
> @excalibur = rw
> @james = rw
> @cocoon = rw
>
> so all cocoon committers can commit to all of excalibur. Cocoon is one
> of the most important users of the excalibur codebase, and we like to
> keep the barrier as low as possible (trusting of course no-one makes a
> mess out of things :-D).
Good to know that!
Best Regards,
Antonio Gallardo
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@excalibur.apache.org
For additional commands, e-mail: dev-help@excalibur.apache.org
Apache Excalibur Project -- URL: http://excalibur.apache.org/
Re: refactoring StoreJanitorImpl
Posted by Leo Simons <ls...@jicarilla.org>.
Giacomo Pati wrote:
> Hy guys
Hi Giacomo,
good to see you around here :-D
> We had the problem over in Cocoon that the StoreJanitoImpl class cannot
> be easily subclassed as we have replaces the deprecated Excalibur event
> package with our own we'd like the StoreJanitorImpl to be used the same
> way wrt thread usage. So we'd like to be able to overwrite the start and
> run method to have our approach to launch regularly background tasks but
> we want the logic to be the same as in the subclass (seams to be
> sofisticated, no need to change or duplicate that).
>
> So my refactoring is that all the logic in the run method is refactored
> out in its own protected method.
>
> WDYT
sure, why not? Go right ahead!
> As I've propably lost my committer right since the move from Avalon to
> Excalibur how about a patch (well, couldn't see a place neither in Jira
> nor Bugzilla to place that patch)?
Nah, you've still got that access! SVN has this configured:
[/excalibur]
@excalibur = rw
@james = rw
@cocoon = rw
so all cocoon committers can commit to all of excalibur. Cocoon is one
of the most important users of the excalibur codebase, and we like to
keep the barrier as low as possible (trusting of course no-one makes a
mess out of things :-D).
As an aside, jira does have excalibur and fortress in there:
http://excalibur.apache.org/issue-tracking.html
cheers,
Leo
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@excalibur.apache.org
For additional commands, e-mail: dev-help@excalibur.apache.org
Apache Excalibur Project -- URL: http://excalibur.apache.org/