Why synchronize keyword

[no_ejb]

Hi I am fairly new to JBoss(started last Friday). I have successfully created my first servlet using Jetty and JBoss 3.0. It worked charmingly!

Just out of curiosity, I started to look at the JBoss source code since I was wondering how JBoss generated session id. But by looking at files under:
org/jboss/jett/session there are plenty of places in the code using synchronized keyword instead of using synchronized version collection(let alone the question why need to synchronize in couple of places). I am sort of disappointed the coding quality at least on multi-thread control level.

It really put my doubt on deploying JBoss in my production web environment where could potentially serving heavy load.

I have two options here: one trust only this particular piece of code is less optimal and correct this particular package. Two use much more expensive tool(such as Weblogic) which I could careless about synchronization issue, it becomes their problem.

I think I might just start as approach one. So the solution is very simple just get rid of synchronize keyword and start using Doug Lea's concurrent package which I used a lot and prove he knows multi-threading much more than I do.

What do you think?

[jules]

It's my code that you are criticising (probably).

The entire distributed session manager has been rewritten for 3.0.1. Unless you are running distributed webapps this would not affect you anyway.

If you are running 3.0 - upgrade.

If you are running 3.0.1 - qualify your remarks with some examples and maybe some constructive suggestion and I will take a look.

I am probably synchronising over the whole collection so that I can iterate over it.


Jules

[gregwilkins]

It is always risky to damn an entire suite from one or
two modules of the code.

Also not that using the synchronized keyword is not always that evil. Often it is better than using synchronized collections.

It is a trade off between holding a lock too long vs
obtaining/releasing the lock too often. The optimal
strategy is often only determined after long hours of analysis. The pluggable session managers are rather new and are probably being a bit conservative at the moment.

But there are definitely plans to provide alternative session implementations. For the persistent/distriubted
implementations - I doubt synchronization is going to
be the bottleneck. For in memory sessions, we already
have plans to use hashmaps that lock at the bucket level rather than the container level.

[no_ejb]

100% agree with all your points.

I get myself in the habit of avoid synchronize on get function call for hashmaps. If I want to iterate through the collection, I will make a fail-safe iterator rather than lock entire collection to do update. Lock entire collection for read purpose is not optimal at all since the overhead is not gaining/release lock, it is loop overhead. Lock on the bucket level is a very good approach(could be part of fail-safe iterator).

Synchronize keyword is evil when there is a deadlock and you will need to find zillions of places in the code trying to figure out where is holdup.

In my experience starting with less synchronization after we find the problem then tight up the synchronization will be the way to go.

By no means I want to bash this product. Quite contrary I like to see JBoss be successful, that is why I am picky on it otherwise I will be indifference.

By the way, I am using 3.0.0 release version. Maybe the latest version is different than what I saw in this module.

[jules]

My approach, which will explain our different views on my code, is to initially write everything to be simple and 100% functional, then go back and optimise the pieces which prove to be bottlenecks.

Often these are not where you think they are going to be.

If I choose to rewrite a piece of code whilst it is in development, I am not throwing away something that I have spent time optimising.

The code, whilst sub-optimal, is immediately usable.

Does that bring us any closer to an understanding ?


Jules