Hi,
Well, since you are right I think I should worry
at least a little bit...
For the timeout code, this cannot really be understood
unless the underlying data structure and algorithm is
understood. For this I am adding a large comment that
explains it with some pseudocode.
And the multithreading issues are also not obvious, so
I am going to add a comment describing this too.
I have used a lot of calls to an assert() method. This
may be viewed as a kind of comment: If you see assert(expr)
you can read it as a comment "expr must evaluate to true
here.". The advantage of assert() and the other checking
methods I use is that they do the check at runtime
whenever the code is executed, and if a check fails we
get an "Assertion failed" exception instead of undefined
behaviour. The downside of this is of course that it
slows down the code at runtime. But when we are absolutely
sure that the code works OK I an going to comment out
all these checks.
I am currently pretty sure that there are no bugs in the
org.jboss.util.timeout module. But we have seen some
problems where timeouts stop happening. Currently there
is only a single timeout callback thread so if this thread
hangs during a timeout calback we get no other timeouts.
To fix this I am changing the code to start a new thread
for every timeout callback. Since timeouts should be rare
we shouldn't worry about the overhead of this.
I will commit these changes as soon as I get over my
problem with the jbosstest testbeantest...
Best Regards,
Ole Husgaard.
marc fleury wrote:
>
> It was a friendly slap don't worry about it.
>
> For the perf vs beauty, it is more a readability issue more than anything
> else. Remember that you are now writing for open source and the readability
> is EXTREMELLY important. There seems to still be bugs in this stuff and I
> like to go in and "read a story" as I stroll through code. Even rickard is
> doing better these days and uses comments at every line which usually
> indicates that you fully understand what you are doing and it's CONTEXT.
> When I read your code it is tough to see the thread clearly, but as long as
> it works I guess I am ok with it...
>
> Readability and simplicity over fancy and fast, at least in our first
> implementations.
> Don't underestimate the power of XP and peer review, so make it simple for
> your peers to read the code and you will see the speed come from it quite
> naturally...
>
> marc
>
> |-----Original Message-----
> |From: [EMAIL PROTECTED]
> |[mailto:[EMAIL PROTECTED]]On Behalf Of Ole Husgaard
> |Sent: Tuesday, October 10, 2000 7:33 AM
> |To: jBoss Developer
> |Subject: [jBoss-Dev] Re: [jBoss-User] Container exception.
> |Notifythecontainerdevelopers:-)
> |
> |
> |Hi,
> |
> |Moving this to jboss-dev...
> |
> |marc fleury wrote:
> |>
> |> ole,
> |>
> |> <just a friendly slap>
> |> you are obviously a C coder by background ;-)
> |
> |Not really. Less than 10% of the code I've produced was written
> |in C, and I have almost no experience in C++ (too ugly for my
> |taste). And I probably knew about a dozen other languages
> |(including Smalltalk) when I wrote my first "Hello, world" in C.
> |
> |> please try to do some java stuff, and pay attention to details
> |
> |Problem probably is that I have been very concerned about
> |performance in the code I've contributed so far.
> |
> |For the TxCapsule change from collections to arrays: I have to
> |admit that this is ugly compared to the performance gained.
> |But it is all hidden in private fields and methods.
> |
> |For the timeout code: The Java collection classes do not have
> |a data structure implementing a priority queue, so I had to
> |write it myself. And by not factoring the priority queue out into
> |a seperate classs, I am able to do timeout cancels in time
> |O(log(n)). Since then I have learned that JavaSoft used the same
> |algorithm in the java.util.Timer code, except they _did_ factor
> |the priority queue out into another class so that individual
> |timers cannot be cancelled in an efficient way.
> |But all of this is hidden in private fields and methods in the
> |org.jboss.util.timeout.TimeoutFactory class.
> |IMHO this hairy algoritm is needed for jBoss to scale well.
> |
> |> </just>
> |>
> |> good work though ;-) just pay attention and simplify your style :-P
> |
> |I think is important to differentiate between the interface
> |and the implementation.
> |
> |For interfaces I am _very_ concerned about simplicity and
> |ease of use. You have probably noted my habit of restricting
> |scope as much as possible. I guess the advantages of this
> |are obvious. If some hairy implementation detail is to be
> |exported through the interface, I consider very carefully first.
> |An example of this is the requirement that a Timeout reference
> |should never be used after it has been cancelled. The disadvantage
> |of this is that some timeout user may not respect that
> |requirement, but the advantage is that Timeout objects may be
> |reused thus lessening the burden on the allocator and the garbage
> |collector. I could have made a similar requirement that a Timeout
> |reference never be used after the timeout happened (thus enabling
> |reuse of Timeout objects after timeouts too), but the users would
> |then have to consider races between timeouts and timeout cancels,
> |and that would IMO be too error-phrone.
> |
> |For the implementation I care a lot less about ugly code. The
> |code should IMHO be as simple and easily readable as possible
> |while still doing the needed job with the needed efficiency.
> |But if ugly code in the implementation is needed to get the job
> |done with the needed efficiency, I'll accept ugly code. But I
> |always try to isolate the ugliness as much as possible.
> |
> |
> |Sorry for this lengthy answer. I know this was just a friendly
> |slab, but I think the issue of simplicity versus performance is
> |important.
> |
> |Comments anyone?
> |
> |
> |Best Regards,
> |
> |Ole Husgaard.
> |
> |
