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

[Gerard Ziemski]

Thank you Robbin for addressing all my concerns and answering all my questions. I will probably have more later, but we can handle that then.

Very impressive hashtable.


cheers


> On May 11, 2018, at 2:54 PM, Robbin Ehn <robbin.ehn at oracle.com> wrote:
> 
> 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