commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From James Carman <jcar...@carmanconsulting.com>
Subject Re: [Graph] Test problems after last commit
Date Sat, 03 Mar 2012 13:40:07 GMT
I think a read/write lock might help.  Reads only need to be synchronized
around writes.  They don't have to be synchronized with one another.
On Mar 3, 2012 8:35 AM, "Simone Tripodi" <simonetripodi@apache.org> wrote:

> Hola Marco,
>
> > my doubt is this:  opening a synchronized block into handler
> implementation on 'this',  in my opinion is not enough, because the method
> "CommonsGraph.synchronize()" returns a instance of the Proxy and not an
> instance of handler.
>
> true, but *all* methods executions (synchronized and not) are
> performed inside the handler, contained in the proxy instance anyway.
>
> So an user cannot use the same "lock"  in order to synchronize a block
> like this:
>
> ====
> Graph g = CommonsGraph.synchronize(g_);
>   ...
>  synchronized(g) {
>     for ( BaseLabeledVertex v2 : g.getVertices() )
>     {
>         // do somethings
>     }
>  }
> ====
>
> I am not sure we can do much more of what we've already done, for that
> situation: the snippet below mixes the *data structure*
> synchronization with the business logic performed outside.
>
> > So my idea is to open synchronized block inside handler implementation
> using the same Proxy instance that the method 'CommonsGraph.synchronize'
> will return.
> > Furthermore I'd like to modify the API by adding also the possibility
> for the user to use an your own lock, in that way
> >
> > CommonsGraph.synchronize( G graph, Object lock )
>
> I have not clear which benefit you want to obtain with that - looks at
> Collections#synchronized* methods and see that the separation of
> concerns in clear.
>
> > Finally only one question. I think that we should add some tests in
> order to check the correct implementation of our multithrading
> implementation.
> > Do you think that we can use an external library in test scope?
>
> Sure that we can, the only issue we have is that GroboUtils is not in
> the central repo, adding the external repository would make not easy
> having the component released.
>
> best,
> -Simo
>
> http://people.apache.org/~simonetripodi/
> http://simonetripodi.livejournal.com/
> http://twitter.com/simonetripodi
> http://www.99soft.org/
>
>
>
> On Sat, Mar 3, 2012 at 2:13 PM, Marco Speranza <marcosperanza@apache.org>
> wrote:
> > Morning Simo,
> >
> > my doubt is this:  opening a synchronized block into handler
> implementation on 'this',  in my opinion is not enough, because the method
> "CommonsGraph.synchronize()" returns a instance of the Proxy and not an
> instance of handler.
> > So an user cannot use the same "lock"  in order to synchronize a block
> like this:
> >
> > ====
> > Graph g = CommonsGraph.synchronize(g_);
> >    ...
> >  synchronized(g) {
> >      for ( BaseLabeledVertex v2 : g.getVertices() )
> >      {
> >          // do somethings
> >      }
> >  }
> > ====
> >
> > So my idea is to open synchronized block inside handler implementation
> using the same Proxy instance that the method 'CommonsGraph.synchronize'
> will return.
> > Furthermore I'd like to modify the API by adding also the possibility
> for the user to use an your own lock, in that way
> >
> > CommonsGraph.synchronize( G graph, Object lock )
> >
> > WDYT?
> >
> > Finally only one question. I think that we should add some tests in
> order to check the correct implementation of our multithrading
> implementation.
> > Do you think that we can use an external library in test scope?
> >
> >
> > have a nice day
> >
> >
> > --
> > Marco Speranza <marcosperanza@apache.org>
> > Google Code: http://code.google.com/u/marco.speranza79/
> >
> > Il giorno 03/mar/2012, alle ore 13:50, Simone Tripodi ha scritto:
> >
> >> Good morning Marco,
> >>
> >> I had the chance to have a deeper look at your yesterday's night work
> >> and think your additions are good improvements - I just wonder if we
> >> can replace the lock object with the handler itself, referencing
> >> `this` instead.
> >>
> >> Thoughts?
> >>
> >> best and have a nice WE,
> >> -Simo
> >>
> >> http://people.apache.org/~simonetripodi/
> >> http://simonetripodi.livejournal.com/
> >> http://twitter.com/simonetripodi
> >> http://www.99soft.org/
> >>
> >>
> >>
> >> On Sat, Mar 3, 2012 at 1:56 AM, Simone Tripodi <
> simonetripodi@apache.org> wrote:
> >>> I saw the commit, please read comments inline.
> >>> best,
> >>> -Simo
> >>>
> >>> http://people.apache.org/~simonetripodi/
> >>> http://simonetripodi.livejournal.com/
> >>> http://twitter.com/simonetripodi
> >>> http://www.99soft.org/
> >>>
> >>>
> >>>
> >>> On Sat, Mar 3, 2012 at 1:40 AM, Marco Speranza <
> marcosperanza@apache.org> wrote:
> >>>> Hi I fixed the problem using proxy/handler.
> >>>>
> >>>> I put also a couple of tests. For do that I insert a test scoped
> dependency to a library net.sourceforge.groboutils.groboutils-core
> >>>>
> >>>> Ciao
> >>>>
> >>>> --
> >>>> Marco Speranza <marcosperanza@apache.org>
> >>>> Google Code: http://code.google.com/u/marco.speranza79/
> >>>>
> >>>> Il giorno 03/mar/2012, alle ore 01:29, Simone Tripodi ha scritto:
> >>>>
> >>>>> yes, what I didn't understand is how you would like to manage the
> >>>>> iterators issue
> >>>>>
> >>>>> sorry for the brevity but tonight I am getting crazy with at least
3
> >>>>> other projects :D
> >>>>>
> >>>>> -Simo
> >>>>>
> >>>>> http://people.apache.org/~simonetripodi/
> >>>>> http://simonetripodi.livejournal.com/
> >>>>> http://twitter.com/simonetripodi
> >>>>> http://www.99soft.org/
> >>>>>
> >>>>>
> >>>>>
> >>>>> On Sat, Mar 3, 2012 at 1:16 AM, Marco Speranza <
> marcosperanza@apache.org> wrote:
> >>>>>> I think that we have to use the same patter of java Collections:
a
> wrapper of Graph/MutableGraph that use a synchronize block.
> >>>>>>
> >>>>>> WDYT?
> >>>>>>
> >>>>>> --
> >>>>>> Marco Speranza <marcosperanza@apache.org>
> >>>>>> Google Code: http://code.google.com/u/marco.speranza79/
> >>>>>>
> >>>>>> Il giorno 03/mar/2012, alle ore 01:10, Simone Tripodi ha scritto:
> >>>>>>
> >>>>>>> OK now sounds better, thanks - how would you intend to fix
it?
> >>>>>>> TIA,
> >>>>>>> -Simo
> >>>>>>>
> >>>>>>> http://people.apache.org/~simonetripodi/
> >>>>>>> http://simonetripodi.livejournal.com/
> >>>>>>> http://twitter.com/simonetripodi
> >>>>>>> http://www.99soft.org/
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>> On Sat, Mar 3, 2012 at 1:01 AM, Marco Speranza <
> marcosperanza@apache.org> wrote:
> >>>>>>>>>>
> >>>>>>>>>> furthermore there is another problem: with handler
is not
> possible to use correctly synchronization block like this
> >>>>>>>>>>
> >>>>>>>>>> ====
> >>>>>>>>>> Graph g = CommonsGraph.synchronize(g_);
> >>>>>>>>>>     ...
> >>>>>>>>>>  synchronized(g) {
> >>>>>>>>>>       for ( BaseLabeledVertex v2 : g.getVertices()
)
> >>>>>>>>>>       {
> >>>>>>>>>>           // do somethings
> >>>>>>>>>>       }
> >>>>>>>>>>  }
> >>>>>>>>>> ====
> >>>>>>>>>
> >>>>>>>>> sorry I don't understand what you meant/intend to
do
> >>>>>>>>
> >>>>>>>> I'm trying to explain better. the method getVertices
return an
> Iterator. In a multi-thread environment you have to ensure that during the
> 'for' execution the [graph] data structure remains  consistent.
> >>>>>>>> Also for java collection is the same [
> http://docs.oracle.com/javase/6/docs/api/java/util/Collections.html#synchronizedCollection(java.util.Collection)
> ]
> >>>>>>>>
> >>>>>>>> So if we use a proxy/handler there is no way to "export"
the
> mutex/lock variable used into handler class.
> >>>>>>>>
> >>>>>>>> In  our CommonsGraph the variable this.lock is private
and is non
> possible to export out of the method, so the user can not utilize the lock
> to synchronize his block.
> >>>>>>>>
> >>>>>>>> ===
> >>>>>>>>        public Object invoke( Object proxy, Method method,
> Object[] args )
> >>>>>>>>            throws Throwable
> >>>>>>>>        {
> >>>>>>>>            if ( synchronizedMethods.contains( method
) )
> >>>>>>>>            {
> >>>>>>>>                try
> >>>>>>>>                {
> >>>>>>>>                    synchronized ( this.lock )
> >>>>>>>>                    {
> >>>>>>>>                        return method.invoke( synchronizedMethods,
> args );
> >>>>>>>>                    }
> >>>>>>>>                }
> >>>>>>>>                catch ( InvocationTargetException e )
> >>>>>>>>                {
> >>>>>>>>                    throw e.getTargetException();
> >>>>>>>>                }
> >>>>>>>>            }
> >>>>>>>>            return method.invoke( synchronizedMethods,
args );
> >>>>>>>>        }
> >>>>>>>> ===
> >>>>>>>>
> >>>>>>>> ciao
> >>>>>>>>
> >>>>>>>> --
> >>>>>>>> Marco Speranza <marcosperanza@apache.org>
> >>>>>>>> Google Code: http://code.google.com/u/marco.speranza79/
> >>>>>>>>
> >>>>>>>> Il giorno 03/mar/2012, alle ore 00:45, Simone Tripodi
ha scritto:
> >>>>>>>>
> >>>>>>>>>> furthermore there is another problem: with handler
is not
> possible to use correctly synchronization block like this
> >>>>>>>>>>
> >>>>>>>>>> ====
> >>>>>>>>>> Graph g = CommonsGraph.synchronize(g_);
> >>>>>>>>>>     ...
> >>>>>>>>>>  synchronized(g) {
> >>>>>>>>>>       for ( BaseLabeledVertex v2 : g.getVertices()
)
> >>>>>>>>>>       {
> >>>>>>>>>>           // do somethings
> >>>>>>>>>>       }
> >>>>>>>>>>  }
> >>>>>>>>>> ====
> >>>>>>>>>
> >>>>>>>>> sorry I don't understand what you meant/intend to
do
> >>>>>>>>>
> >>>>>>>>>> I really think that a synchronized wrapper it's
the best
> solution.
> >>>>>>>>>>
> >>>>>>>>>> WDYT?
> >>>>>>>>>
> >>>>>>>>> they sucks because we have to keep them updated
while , but if
> there
> >>>>>>>>> are not alternatives...
> >>>>>>>>> -Simo
> >>>>>>>>>
> >>>>>>>>> http://people.apache.org/~simonetripodi/
> >>>>>>>>> http://simonetripodi.livejournal.com/
> >>>>>>>>> http://twitter.com/simonetripodi
> >>>>>>>>> http://www.99soft.org/
> >>>>>>>>>
> >>>>>>>>>
> ---------------------------------------------------------------------
> >>>>>>>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> >>>>>>>>> For additional commands, e-mail: dev-help@commons.apache.org
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> ---------------------------------------------------------------------
> >>>>>>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> >>>>>>>> For additional commands, e-mail: dev-help@commons.apache.org
> >>>>>>>>
> >>>>>>>
> >>>>>>>
> ---------------------------------------------------------------------
> >>>>>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> >>>>>>> For additional commands, e-mail: dev-help@commons.apache.org
> >>>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> ---------------------------------------------------------------------
> >>>>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> >>>>>> For additional commands, e-mail: dev-help@commons.apache.org
> >>>>>>
> >>>>>
> >>>>> ---------------------------------------------------------------------
> >>>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> >>>>> For additional commands, e-mail: dev-help@commons.apache.org
> >>>>>
> >>>>
> >>>>
> >>>> ---------------------------------------------------------------------
> >>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> >>>> For additional commands, e-mail: dev-help@commons.apache.org
> >>>>
> >>
> >> ---------------------------------------------------------------------
> >> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> >> For additional commands, e-mail: dev-help@commons.apache.org
> >>
> >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> > For additional commands, e-mail: dev-help@commons.apache.org
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message