RFR(L): 8195098: Low latency hashtable for read-mostly scenarios

[Robbin Ehn]

Hi Gerard,

On 2018-05-10 19:55, Gerard Ziemski wrote:
> hi Robbin,
> 
> There are 2 minor issues that caught my eye, anything else we can fix in followup issues:
> 
> #1
> 
>   // This method looks on the _invisible_epoch field and
>   // does a write_synchronize if needed.
>   void write_synchonize_on_visible_epoch(Thread* thread);
> 
> 
> a) I think in the word “looks” in the comment should be “locks”

I don't like lock, sine it implies mutual exclusiveness. I added an explanation, 
hopefully you like it.

> b) Why is the method name using word “visible” when it locks on an “invisible” epoch?

The method only does write_synchronize if the the 'epoch'/generation was seen by 
another thread. Hence write_synchonize_on_visible_epoch, e.g. it only does
wite_synchronize if hashtable state was publish to another thread. Hopefully my
explanation added above makes it much clearer.

> 
> 
> #2 ScopedCS and MultiGetHandle look identical?

ScopedCS is an internal class only and MultiGetHandle an interface class.
But since they provide same type scoped with identical code I let
MultiGetHandle inherit from ScopedCS instead and removed the code duplication, 
thanks!

Please see incremental:
http://cr.openjdk.java.net/~rehn/8195098/v3/inc/

For reference full:
http://cr.openjdk.java.net/~rehn/8195098/v3/full/webrev/

Thanks, Robbin

> 
> // ScopedCS
> template <typename VALUE, typename CONFIG, MEMFLAGS F>
> inline ConcurrentHashTable<VALUE, CONFIG, F>::
>   ScopedCS::ScopedCS(Thread* thread, ConcurrentHashTable<VALUE, CONFIG, F>* cht)
>     : _thread(thread), _cht(cht)
> {
>   GlobalCounter::critical_section_begin(_thread);
>   // This version is published now.
>   if (OrderAccess::load_acquire(&_cht->_invisible_epoch) != NULL) {
>     OrderAccess::release_store_fence(&_cht->_invisible_epoch, (Thread*)NULL);
>   }
> }
> 
> template <typename VALUE, typename CONFIG, MEMFLAGS F>
> inline ConcurrentHashTable<VALUE, CONFIG, F>::
>   ScopedCS::~ScopedCS()
> {
>   GlobalCounter::critical_section_end(_thread);
> }
> 
> // MultiGetHandle
> template <typename VALUE, typename CONFIG, MEMFLAGS F>
> inline ConcurrentHashTable<VALUE, CONFIG, F>::
>   MultiGetHandle::MultiGetHandle(Thread* thread,
>                                  ConcurrentHashTable<VALUE, CONFIG, F>* cht)
>     : _thread(thread), _cht(cht)
> {
>   GlobalCounter::critical_section_begin(_thread);
>   // This version is published now.
>   if (OrderAccess::load_acquire(&_cht->_invisible_epoch) != NULL) {
>     OrderAccess::release_store_fence(&_cht->_invisible_epoch, (Thread*)NULL);
>   }
> }
> 
> template <typename VALUE, typename CONFIG, MEMFLAGS F>
> inline ConcurrentHashTable<VALUE, CONFIG, F>::
>   MultiGetHandle::~MultiGetHandle()
> {
>   GlobalCounter::critical_section_end(_thread);
> }
> 
> 
> cheers
> 
>> On May 2, 2018, at 4:03 AM, Robbin Ehn <robbin.ehn at oracle.com> wrote:
>>
>> Hi all,
>>
>> Here is an update with Gerard's and Coleen's input.
>>
>> Inc:
>> http://cr.openjdk.java.net/~rehn/8195098/v1/inc/webrev/
>> Full:
>> http://cr.openjdk.java.net/~rehn/8195098/v1/full/webrev/
>>
>> Thanks, Robbin
>