activemq-users mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Tim Bain <tb...@alumni.duke.edu>
Subject Re: Advice for Scheduler refactor.
Date Thu, 07 May 2015 13:30:03 GMT
I agree with your approach with the WeakRunnable; I think that will achieve
the goal without the performance hit of calling purge() after each
cancellation.

If you take on the synchronized keywords, you should consider whether we're
worried about concurrent calls to doStart() and doStop().  That scenario
could leak a Timer, and the current code doesn't protect against that
(you'd need a flag to say whether you've been stopped, and you'd need to
check it in doStart() ).  It's the only reason I see that we might want to
synchronize those two methods.  For the other three, make sure Timer itself
is threadsafe, not just the Map, before you remove the synchronized
keywords.

Tim
On May 2, 2015 5:42 PM, "Kevin Burton" <burton@spinn3r.com> wrote:

> Wanted some feedback on this.
>
> https://gist.github.com/burtonator/34a67c24ca9ce0574c04
>
> I think I want to refactor the cancel method…
>
> it calls purge() which is VERY expensive on large numbers of queues. N^2
> expensive.
>
> once the cancel() is called, the timer task won’t get executed, HOWEVER
> there’s still a reference to the Runnable.  This is why I think purge() is
> called.  This allows GC to kick in and remove the runnable without waiting
> for the timer to fire.  Which I think is just 30 seconds later but not
> sure.
>
> So I think the workaround here is to not to submit the Runnable itself but
> a WeakRunnable and then, once we read the TimerTask, we just set the
> reference to the underlying runnable to null.  This will allow GC and we
> also don’t have to call purge().
>
> Ps.  There are some other things that should be cleaned up here.  Probably
> removing synchronized and using ConcurrentHashMap and also probably
> ditching Timer and moving to Executors.. but that’s another project in and
> of itself and might introduce bugs for no win.
>
> Kevin
>
> --
>
> Founder/CEO Spinn3r.com
> Location: *San Francisco, CA*
> blog: http://burtonator.wordpress.com
> … or check out my Google+ profile
> <https://plus.google.com/102718274791889610666/posts>
>

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