What's the purpose of hashtable::reverse()

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

What's the purpose of hashtable::reverse()

Ioi Lam
This seems to be used only by CDS dumping, and if I remove the call
nothing bad seems to happen. Does anyone know what hashtable::reverse()
could possibly achieve?

Also, there's another variant that seems to be dead code that no one calls.

     template <class T, MEMFLAGS F> void Hashtable<T, F>::reverse(void*
boundary)

- Ioi


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: What's the purpose of hashtable::reverse()

Mikael Gerdin
Hi Ioi,

I found this old bug where I think the reversing was added:
https://bugs.openjdk.java.net/browse/JDK-4994483

/Mikael

On 2017-03-16 07:48, Ioi Lam wrote:

> This seems to be used only by CDS dumping, and if I remove the call
> nothing bad seems to happen. Does anyone know what hashtable::reverse()
> could possibly achieve?
>
> Also, there's another variant that seems to be dead code that no one calls.
>
>     template <class T, MEMFLAGS F> void Hashtable<T, F>::reverse(void*
> boundary)
>
> - Ioi
>
>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: What's the purpose of hashtable::reverse()

Ioi Lam
Thanks Mikael for the detective work. The bug comment says:

"the String and symbol tables are hashtables where the hashbuckets
are linked lists -- in reverse chronological order of there creation. This
means that, in some circumstances, some symbols are unnecessarily touched
when looking for older most likely needed symbols. This can be addressed by
simply reversing the order of the lists before creating the archive."

Since JDK9, CDS no longer stores the string and symbol tables using the
"regular" hashtable.

The only "regular" hashtable use by CDS is the shared dictionary (for
name->class lookup), but this table is rehashed at some point, so
calling reverse() will no longer has the desired effect of moving
popular Klasses (such as java/lang/Object) to the front of a linked-list.

So this optimization is irrelevant now. I've created an RFE to remove it.

https://bugs.openjdk.java.net/browse/JDK-8176863

Thanks
- Ioi

On 3/16/17 5:17 PM, Mikael Gerdin wrote:

> Hi Ioi,
>
> I found this old bug where I think the reversing was added:
> https://bugs.openjdk.java.net/browse/JDK-4994483
>
> /Mikael
>
> On 2017-03-16 07:48, Ioi Lam wrote:
>> This seems to be used only by CDS dumping, and if I remove the call
>> nothing bad seems to happen. Does anyone know what hashtable::reverse()
>> could possibly achieve?
>>
>> Also, there's another variant that seems to be dead code that no one
>> calls.
>>
>>     template <class T, MEMFLAGS F> void Hashtable<T, F>::reverse(void*
>> boundary)
>>
>> - Ioi
>>
>>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: What's the purpose of hashtable::reverse()

coleen.phillimore


On 3/16/17 6:20 AM, Ioi Lam wrote:

> Thanks Mikael for the detective work. The bug comment says:
>
> "the String and symbol tables are hashtables where the hashbuckets
> are linked lists -- in reverse chronological order of there creation.
> This
> means that, in some circumstances, some symbols are unnecessarily touched
> when looking for older most likely needed symbols. This can be
> addressed by
> simply reversing the order of the lists before creating the archive."
>
> Since JDK9, CDS no longer stores the string and symbol tables using
> the "regular" hashtable.
>
> The only "regular" hashtable use by CDS is the shared dictionary (for
> name->class lookup), but this table is rehashed at some point, so
> calling reverse() will no longer has the desired effect of moving
> popular Klasses (such as java/lang/Object) to the front of a linked-list.
>
> So this optimization is irrelevant now. I've created an RFE to remove it.

Great, thank you!

More optimizations that we have no means of measurement.

Coleen

>
> https://bugs.openjdk.java.net/browse/JDK-8176863
>
> Thanks
> - Ioi
>
> On 3/16/17 5:17 PM, Mikael Gerdin wrote:
>> Hi Ioi,
>>
>> I found this old bug where I think the reversing was added:
>> https://bugs.openjdk.java.net/browse/JDK-4994483
>>
>> /Mikael
>>
>> On 2017-03-16 07:48, Ioi Lam wrote:
>>> This seems to be used only by CDS dumping, and if I remove the call
>>> nothing bad seems to happen. Does anyone know what hashtable::reverse()
>>> could possibly achieve?
>>>
>>> Also, there's another variant that seems to be dead code that no one
>>> calls.
>>>
>>>     template <class T, MEMFLAGS F> void Hashtable<T, F>::reverse(void*
>>> boundary)
>>>
>>> - Ioi
>>>
>>>
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: What's the purpose of hashtable::reverse()

Mikael Gerdin
Hi Coleen,

On 2017-03-16 16:52, [hidden email] wrote:

>
>
> On 3/16/17 6:20 AM, Ioi Lam wrote:
>> Thanks Mikael for the detective work. The bug comment says:
>>
>> "the String and symbol tables are hashtables where the hashbuckets
>> are linked lists -- in reverse chronological order of there creation.
>> This
>> means that, in some circumstances, some symbols are unnecessarily touched
>> when looking for older most likely needed symbols. This can be
>> addressed by
>> simply reversing the order of the lists before creating the archive."
>>
>> Since JDK9, CDS no longer stores the string and symbol tables using
>> the "regular" hashtable.
>>
>> The only "regular" hashtable use by CDS is the shared dictionary (for
>> name->class lookup), but this table is rehashed at some point, so
>> calling reverse() will no longer has the desired effect of moving
>> popular Klasses (such as java/lang/Object) to the front of a linked-list.
>>
>> So this optimization is irrelevant now. I've created an RFE to remove it.
>
> Great, thank you!
>
> More optimizations that we have no means of measurement.

That was stated in the evaluation of this issue some 13 years ago (time
flies ;)

> ###@###.### 2004-02-13
> The measured performance benefit of these changes is less than expected and
> it will be difficult to justify changes to tiger this late without better
> numbers.

/Mikael

>
> Coleen
>>
>> https://bugs.openjdk.java.net/browse/JDK-8176863
>>
>> Thanks
>> - Ioi
>>
>> On 3/16/17 5:17 PM, Mikael Gerdin wrote:
>>> Hi Ioi,
>>>
>>> I found this old bug where I think the reversing was added:
>>> https://bugs.openjdk.java.net/browse/JDK-4994483
>>>
>>> /Mikael
>>>
>>> On 2017-03-16 07:48, Ioi Lam wrote:
>>>> This seems to be used only by CDS dumping, and if I remove the call
>>>> nothing bad seems to happen. Does anyone know what hashtable::reverse()
>>>> could possibly achieve?
>>>>
>>>> Also, there's another variant that seems to be dead code that no one
>>>> calls.
>>>>
>>>>     template <class T, MEMFLAGS F> void Hashtable<T, F>::reverse(void*
>>>> boundary)
>>>>
>>>> - Ioi
>>>>
>>>>
>>
>
Loading...