jackrabbit-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Tobias Bocanegra" <tobias.bocane...@day.com>
Subject Re: use of java 1.4 assert
Date Thu, 23 Feb 2006 09:27:21 GMT
hi piyush,
this was a pure performane optimization. it actually _was_ a weak
hashset before. since the itemstate stuff is deeply in the core and
all internal, no everyday jackrabbit user/jsr170 developer should need
to hassle with an assertion exception.

regards, toby

On 2/22/06, Piyush Purang <ppurang@gmail.com> wrote:
> Hi Marcel,
>
> I finally had time to look into your usage of assert and I can't convince
> myself that it is correct usage.
>
> The reason you decided to use an assert-statement is that on a production
> environment (PE) asserts will be turned off (which is a very likely
> scenario) and hence performance will be better as per the promise [1].
>
> The code was
>
>  if (!listeners.containsKey(listener)) {
>            listeners.put(listener, listener);
> and now is
>
> synchronized (listeners) {
>            assert (!listeners.contains(listener));
>            listeners.add (listener);
>
> The previous version silently ignored requests to re-register listeners that
> are already registered. That is the client doesn't have to do any
> bookkeeping he can register umpteen times and he will be delivered the event
> just once.
>
> Now in the new version on the developer's box it will throw an
> AssertionError and the developer would have been strong-muscled into
> following the dictum register just once. Most of the times though Listeners
> that would be registering themselves don't want to hassle theme selves in
> keeping tabs on how many times did they register. (At least I never
> programmed my listeners like that. I just registered.) Hence there is a
> possibility that listeners on the PE get added multiple-times to the
> listener queue and receive the same event many times unless we again do some
> bookkeeping either on our side or listeners do it themselves (Performance
> will be hit). And no listener expects to be told about the same event more
> than once; that is something we all can agree on.
>
> And if you still want to call it a pre-condition then [1] clearly points out
> don't use Assertions to implement pre-conditions for public methods.
>
> Do you agree with my analysis? I of course don't have the kind of overview
> that you have of jackrabbit. Maybe my presumption that the outside world
> will be registering listeners is false. Even in that case I am not 100%
> convinced about using assertions to fit the role.
>
> How about using a weak hash set?
>
> Cheers
> Piyush
>
> [1] http://java.sun.com/j2se/1.4.2/docs/guide/lang/assert.html
>
>  On 2/17/06, Marcel Reutegger <marcel.reutegger@gmx.net> wrote:
> >
> > I've just committed a change that uses asserts in the sources.
> >
> > Users of IntelliJ have to manually change the language level of the
> > jackrabbit project to 1.4. It seems that the idea maven plugin does not
> > take the maven.compile.source=1.4 property into account when generating
> > a project file :-/
> >
> > regards
> >   marcel
> >
> > Marcel Reutegger wrote:
> > > hi all,
> > >
> > > I'm currently working on an improvement regarding memory consumption in
> > > jackrabbit (replace reference map for listener in item states with a
> > > more light weight collection class). doing that, I would like to use the
> > > java 1.4 assert facility to include preconditions that make sure a
> > > listener is only registered once. that way I don't have to check every
> > > time if a listener is already registered.
> > >
> > > using 1.4 asserts would required the '-source 1.4' switch when compiling
> > > jackrabbit. our maven project.properties already adds that switch. so,
> > > there would be nothing to do on our side. but probably a comment in
> > > documentation that talks about this.
> > >
> > > per default asserts are disabled and will not impact performance.
> > > when running the unit tests, asserts would then be enabled using the -ea
> > > switch for the test run.
> > >
> > > what's the general feeling about this?
> > >
> > > regards
> > >  marcel
> > >
> >
> >
>
>


--
-----------------------------------------< tobias.bocanegra@day.com >---
Tobias Bocanegra, Day Management AG, Barfuesserplatz 6, CH - 4001 Basel
T +41 61 226 98 98, F +41 61 226 98 97
-----------------------------------------------< http://www.day.com >---

Mime
View raw message