[hibernate-dev] Hibernate & Java 5 ?

Tue Dec 15 04:46:20 EST 2009

Hello, thanks for your work!

I don't have much to comment except that;
 - we use tabs for indentation
 - logs are a bit verbose but I can work on that when applying the patch (will be faster then explaining everything)
 - are you sure you can statically reference ConcurrentStatisticsImpl wo failing at runtime on all VMs? I don't thing the JLS guarantees that. You might have to load the class via reflection
 - don't use Class.forName, prefer ReflectHelper.classForName
 - anybody knows what svn:executable means?
 - in ConcurrentQueryStatisticsImpl I don't quite understand your read/write lock use. They look backwards. Also the weird for loops in execute are probably worth a comment or two
 - use StringBuilder in toString for the concurrent impls

Alex, can you sign the JBoss contributor agreement (somewhat similar tot he Apache foundation one) https://www.jboss.org/contribute and ping steve afterwards.

I've also commented inline your questions.

On 14 déc. 2009, at 12:13, Alex Snaps wrote:

> Attached is my current diff against trunk r18223 for you guys to review:
> - it introduces org.hibernate.stat.Concurrent* classes that are the JDK5 ones
> - I extracted interfaces for the Collection-, Entity-, Query- and
> SecondLevelCacheStatistics and have their previous impl. in *Impl.
> (the new are Concurrent*Impl)
> - SessionFactory's constructor tries to load AtomicLong and on
> availability uses the new concurrent stats, otherwise falls back to
> the previous impl.
> - Dirty reads are still in the old impl., Yet I'd at least consider
> the StatisticsImpl.isStatisticsEnabled to not be dirty-read and mark
> it as volatile, wdyt?

volatile in 1.4 is meaningless so why bother.

> - As for your HashMap's and the infinite loop trap, I need to look
> into the 1.4 impl. again, the unsynchronized Map.keySet() might be an
> issue indeed
> 
> All the code is Java 1.4 compliant (I removed the generics or
> covariant return types I had in there)
> imho, the impact is not disruptive, but I'll wait for your input to be
> definitive on this :)
> Don't hesitate to ask for some changes, especially if these help
> getting this backported to prior 3.x releases!
> Waiting on your input, and hope I can then submit the final patch on jira
> Thanks,
> Alex
> 
> On Wed, Dec 9, 2009 at 4:58 PM, Steve Ebersole <steve at hibernate.org> wrote:
>> I'm actually ok with just dropping 1.4 support for 3.5.
>> 
>> If you are going to do it, I'd say that "dirty read" here is ok.  What
>> I'd worry about though is concurrency issues like blocking (reads on
>> unsynchronized maps can cause blocks iirc).
>> 
>> As for backporting, well it depends, on a few things.  First, if you
>> want it backported to version prior to 3.5 it will absolutely need some
>> form of 1.4 compatibility.  I think backport-util-concurrent, as you
>> mention, is the best option.  Then its mostly a matter of how disruptive
>> the change is.
>> 
>> On Wed, 2009-12-09 at 14:12 +0100, Alex Snaps wrote:
>>> What about the dirty reads? Should I mark all getters as synchronized
>>> methods, or just leave it as is? Not knowing how intensively these
>>> getters are being called...
>>> Btw, would this be backported into older 3.x releases as well?
>>> 
>>> On Wed, Dec 9, 2009 at 1:39 PM, Emmanuel Bernard <emmanuel at hibernate.org> wrote:
>>>> I don't think it's critical to backport this for 1.4 JDK users. But if you want to spare cycles...
>>>> 
>>>> On 9 déc. 2009, at 12:48, Alex Snaps wrote:
>>>> 
>>>>> I have finished a first version of it all:
>>>>> It is supporting both jdk 1.4 and 1.5+. So that if the
>>>>> java.util.concurrent classes are present, it will use the new
>>>>> ConcurrentStatisticsImpl, otherwise will fallback to the current
>>>>> StatisticsImpl. As mentioned, I had to extract interfaces for
>>>>> EntityStatistics, CollectionStatistics, SecondLevelCacheStatistics and
>>>>> QueryStatistics.
>>>>> Now, there is still the issue of the dirty reads within the current
>>>>> StatisticsImpl. (no synchronization on read)...
>>>>> What would you guys think of fixing these with the
>>>>> backport-util-concurrent ? So that even the 1.4 jdk get better
>>>>> concurrency on these...
>>>>> That would include a new dependency... I'm also currently looking into
>>>>> not including the deps, and using the same tricks as they are doing to
>>>>> diminish contention...
>>>>> Their AtomicLong relies on synchronization, but at least every stat
>>>>> gets its own lock, versus everyone competing for the same one as it is
>>>>> currently the case.
>>>>> wdyt?
>>>>> 
>>>>> On Tue, Dec 1, 2009 at 3:59 PM, Steve Ebersole <steve at hibernate.org> wrote:
>>>>>> I guess I have just been waiting until we can actually leverage 1.5
>>>>>> features (ala utilize enums or expose generics/typing).  That will not
>>>>>> happen for 3.5.
>>>>>> 
>>>>>> Now statistics are encapsulated behind a set of interfaces (Statistics
>>>>>> and StatisticsImplementor).  We could make this alterable like I did for
>>>>>> JDBC 3/4 based on the JVM.  That would mean reflection code though.
>>>>>> 
>>>>>> I do not actually know of any real cases of Hiberate being used in 1.4
>>>>>> environments today.  So maybe we can just make it 1.5 compatible.
>>>>>> 
>>>>>> Votes?
>>>>>> 
>>>>>> 
>>>>>> On Tue, 2009-12-01 at 14:38 +0100, Alex Snaps wrote:
>>>>>>> We've been doing some improvement to the Hibernate statistics at
>>>>>>> Terracotta, when we realized how much the synchronization on it was
>>>>>>> impacting throughput in our tests.
>>>>>>> That is work we wanted to contribute back to you guys, should
>>>>>>> Hibernate Core be target at 1.5. As it seems that's not yet the case,
>>>>>>> so there isn't much you guys will be able to do with these changes...
>>>>>>> We discussed about that at Devoxx with Max and Emmanuel and thought it
>>>>>>> was okay to have 1.5 impl. of the specs (java.util.concurrent based)
>>>>>>> already. Apparently not :( What timeframe do you see 1.4 support being
>>>>>>> dropped?
>>>>>>> 
>>>>>>> On Tue, Dec 1, 2009 at 1:44 PM, Steve Ebersole <steve at hibernate.org> wrote:
>>>>>>>> I have issues reloading Maven-based projects in IntelliJ as well.  I
>>>>>>>> simply try to minimize the number of times I reload.
>>>>>>>> 
>>>>>>>> Hibernate is *built* with JDK 1.5, but not all the modules are 1.5
>>>>>>>> compatible.
>>>>>>>> 
>>>>>>>> What "statistics work" discussion?  I must have missed that.  But for
>>>>>>>> sure the hibernate-core module should remain 1.4 compatible.  Dropping
>>>>>>>> 1.4 support is on the roadmap, but not for 3.5
>>>>>>>> 
>>>>>>>> 
>>>>>>>> On Tue, 2009-12-01 at 11:32 +0100, Alex Snaps wrote:
>>>>>>>>> Hey,
>>>>>>>>> Doing a svn update of the Hibernate trunk, I realized I probably had
>>>>>>>>> changed the project to be Java5 manually as it reverted to 1.4
>>>>>>>>> (because of some pom.xml change) in IntelliJ.
>>>>>>>>> Talking to Max and Emmanuel at Devoxx I thought trunk was now to be
>>>>>>>>> Java 5? Is this not the case after all, or are poms only update when
>>>>>>>>> the first Java5 language/jdk feature sneaks in?
>>>>>>>>> As discussed we discussed, all the statistics work heavily rely on
>>>>>>>>> java.util.concurrent classes, so that is "more or less" important for
>>>>>>>>> that patch...
>>>>>>>>> Btw do you guys have a contributor agreement somewhere, I couldn't find it.
>>>>>>>>> Thanks,
>>>>>>>>> Alex
>>>>>>>>> 
>>>>>>>> --
>>>>>>>> Steve Ebersole <steve at hibernate.org>
>>>>>>>> Hibernate.org
>>>>>>>> 
>>>>>>>> 
>>>>>>> 
>>>>>>> 
>>>>>>> 
>>>>>> --
>>>>>> Steve Ebersole <steve at hibernate.org>
>>>>>> Hibernate.org
>>>>>> 
>>>>>> 
>>>>> 
>>>>> 
>>>>> 
>>>>> --
>>>>> Alex Snaps <alex.snaps at gmail.com>
>>>>> Software Engineer - Terracotta
>>>>> http://twitter.com/alexsnaps
>>>>> http://www.linkedin.com/in/alexsnaps
>>>>> 
>>>>> _______________________________________________
>>>>> hibernate-dev mailing list
>>>>> hibernate-dev at lists.jboss.org
>>>>> https://lists.jboss.org/mailman/listinfo/hibernate-dev
>>>> 
>>>> 
>>> 
>>> 
>>> 
>> --
>> Steve Ebersole <steve at hibernate.org>
>> Hibernate.org
>> 
>> 
> 
> 
> 
> -- 
> Alex Snaps <alex.snaps at gmail.com>
> Software Engineer - Terracotta
> http://twitter.com/alexsnaps
> http://www.linkedin.com/in/alexsnaps
> <HibernateStats.diff>