Virgo47 wrote:
> First I greatly appreciate the article because it let me again to go
> through a few things I did in the Simon core. There is one point I can
> fully agree... sample and map. Map seems so ... easy to process,
> however normal Java Bean is probably as good - or maybe even some
> "Sample" class that can contain some metadata.
Sample class sounds perfect.
> I can also create two
> sample methods - sample() and sampleAndReset() to fix other little
> issues you've found. It would smash magic values problem too.
>
Great!
> For other-than-sample usages I don't like having other object that
> would contain measurement data instead of convenient getters available
> directly on the Simon.
>
Why? Is there a particular use case you have in mind?
I think it keeps the interface nicely small and clean.
> Next thing is thread local in the Stopwatch and more ways how the
> Stopwatch can be used in multithreaded environment. Jamon does the
> same you suggest (sort of) - it creates a facade for monitor every
> time you want it and this frontend holds reference to "monitor data".
> You actually never work with the object that holds the data - only
> with the object that can measure one split in parallel and it holds
> the object that holds the data. ;-)
Hmm, that's I would have done as well...
> Simon is different, Stopwatch is
> heavier but it's only one object for one Stopwatch you work with -
>
Interesting. I'll have to see the code of Jamon to understand why it is
so much slower. Can't be just this, can it?
> ThreadLocal is the price. I don't like the hack you did - I guess it
> was only a concept proof
Yes, that was a proof of concept. To really go with this approach the
interface should change in an incompatible way. Unfortunately.
> - because you can do:
>
> Stopwatch sw = SimonManager.getStopwatch("blabla");
> sw.start();
> ...
> sw.stop();
>
> start returns object for convenience and - although I did not think
> about it that time -
You didn't do that for nothing. It was a sign that there is more going on :)
> I guess it must be this with current interface.
> Other option is that you can obtain Stopwatch either to do something
> with it except to measure, and than you can start it. Start would
> return your proposed object - and in that case it need not to be
> Stopwatch anymore. I may rather see Split object not being confused
> with Stopwatch when it can't do what you expect Stopwatch to do
>
Right, Split is a better word then my proposed StopwatchMeasurement.
Indeed, it should not be a Stopwatch. It should only have the stop
method (although, it is the end of the split, so perhaps 'end' is a
better method name).
> (stopwatch related getters, common Simon functionality).
>
> Now I think Stopwatch is quite consistent which doesn't mean I don't
> see your points. Map & memory leak is something I never thought about.
> "Keyed" start/stop is quite nice way how to tie one measurement to
> some object you can't control (not your implementation), to track its
> lifetime or whatever. ..........
> (Can't this be fixed with some map
> with weaker-references too?)
>
You could use weak references. Problem is that these tend to be heavy
for the GC, you should not use these too often.
> ...... The code
> there is quite... hard to admit... ugly. It does complex stuff ......
> There may be some way how to break the
> code into something better and narrow the synchronized part - but it
> is over my head now.
Hmm, that is not a good sign. It will probably come to you when you let
it rest for a while and then track a few use cases :)
> With synchronized there I feel quite safe - I
> can't imagine proper unit tests to cover concurrency in case of add/
> remove Simons from the tree.
>
It is possible, but hard. Clean Code refers to ConTest, a tool from IBM
that helps you test concurrent code. The 'jiggling' I used in the
performance application also helps.
> Thank you for quite a thorough article - review is always important
> when we want to continue with the Simon to the future. And we do. :-)
> Nice x-mas gift from you.
>
>
Thanks for the open mind!
Regards,
Erik.
--
Erik van Oosten
http://www.day-to-day-stuff.blogspot.com/