On external_word_Relocation and StubCodeDesc

[Vladimir Kozlov]

On 2/16/16 4:30 AM, Vladimir Ivanov wrote:
> On 2/16/16 1:47 AM, Vladimir Kozlov wrote:
>> Actually you could optimize it in case of segmented CodeCache (default
>> with tiered) by checking if PC is in Non-nmethods segment.
> Yes, it should help.
>
> But what I'm trying to understand is how viable is the current scheme (when stub entry points are treated specially). It
> seems stubs are rare among external addresses.
>
> Also, there are other stubs (e.g., produced by Runtime1::generate_blob_for) which aren't put in StubCodeDesc::_list.
>
> So, if it is viable to save on external relocation encoding, we have to come up with a universal solution. Otherwise,
> I'd prefer to keep things simple and completely remove stub-specific logic.
>
>> Also stub's address should not be external since stubs are located in
>> CodeCache. External addresses are used for runtime (or C-heap) addresses.
> I don't think it is always the case, e.g:
>    * many stubs are referenced as ExternalAddress (e.g. [1]);

It is table in C-heap and not a stub in codecache. It is not on StubCodeDesc::_list.

>
>    * range_check [2] issues external addresses, but SharedRuntime::gen_i2c_adapter passes in addresses within the code
> cache [3].

First, I think it is mistake to use ExternalAddress in range_check.
And second none of those addresses are present in StubCodeDesc::_list.
I think your experiment with hit rate is confirmation that StubCodeDesc::desc_for() will find nothing when called from 
external_word_Relocation::pack_data_to(). May be we should path flag as parameter to avoid calling 
StubCodeDesc::desc_for() in such case.

Thanks,
Vladimir

>
> Best regards,
> Vladimir Ivanov
>
> [1] http://hg.openjdk.java.net/jdk9/hs-comp/hotspot/file/9cf33e51c2d4/src/cpu/x86/vm/macroAssembler_x86.cpp#l10233
>
> [2] http://hg.openjdk.java.net/jdk9/hs-comp/hotspot/file/9cf33e51c2d4/src/cpu/x86/vm/sharedRuntime_x86_64.cpp#l683
>
> [3] http://hg.openjdk.java.net/jdk9/hs-comp/hotspot/file/9cf33e51c2d4/src/cpu/x86/vm/sharedRuntime_x86_64.cpp#l731
>
>>
>> Thanks,
>> Vladimir K
>>
>> On 2/15/16 7:37 AM, Vladimir Ivanov wrote:
>>> While working on JDK-8138922 [1], I noticed that StubCodeDesc::desc_for
>>> is called for every external address embedded in the code [2]. It is
>>> done to save some space on relocations (record an index in the stub list
>>> instead of full address).
>>>
>>> StubCodeDesc::desc_for does linear search over the list and there are
>>> around 100 stubs registered on x64.
>>>
>>> The problem is that the hit rate is very low. My experiments (on Octane
>>> [3]) show that there are thousands of requests with 0% hit rate.
>>>
>>> So, instead of speeding up the search (switch to array, sort it and do
>>> binary search to compute the index or lookup the address by index), I
>>> suggest to completely remove that logic and always record the address
>>> for external_word relocations.
>>>
>>> What's your take on that?
>>>
>>> Best regards,
>>> Vladimir Ivanov
>>>
>>> PS: there are some prospects in compacting some "well-known" addresses
>>> [4], but it seems stubs aren't good candidates for that.
>>>
>>> [1] https://bugs.openjdk.java.net/browse/JDK-8138922
>>>
>>> http://mail.openjdk.java.net/pipermail/hotspot-dev/2016-February/021702.html
>>>
>>>
>>>
>>> [2] hotspot/src/share/vm/code/relocInfo.cpp:
>>> void external_word_Relocation::pack_data_to(CodeSection* dest) {
>>>    short* p = (short*) dest->locs_end();
>>>    int32_t index = runtime_address_to_index(_target);
>>>
>>> where runtime_address_to_index looks for the _target among stubs
>>> registered in StubCodeDescs::_list:
>>>
>>> int32_t Relocation::runtime_address_to_index(address runtime_address) {
>>>    assert(!is_reloc_index((intptr_t)runtime_address), "must not look
>>> like an index");
>>>
>>>    if (runtime_address == NULL)  return 0;
>>>
>>>    StubCodeDesc* p = StubCodeDesc::desc_for(runtime_address);
>>>    if (p != NULL && p->begin() == runtime_address) {
>>>      assert(is_reloc_index(p->index()), "there must not be too many
>>> stubs");
>>>      return (int32_t)p->index();
>>>
>>> [3] perf counters:
>>> sun.ci.stubCodeDesc::total_reqs=11788
>>> sun.ci.stubCodeDesc::total_reqs_hit=0
>>>
>>> sun.ci.stubCodeDesc::total_reqs=4117
>>> sun.ci.stubCodeDesc::total_reqs_hit=0
>>>
>>> sun.ci.stubCodeDesc::total_reqs=3270
>>> sun.ci.stubCodeDesc::total_reqs_hit=0
>>>
>>> sun.ci.stubCodeDesc::total_reqs=5183
>>> sun.ci.stubCodeDesc::total_reqs_hit=0
>>>
>>> sun.ci.stubCodeDesc::total_reqs=7001
>>> sun.ci.stubCodeDesc::total_reqs_hit=0
>>>
>>> sun.ci.stubCodeDesc::total_reqs=2909
>>> sun.ci.stubCodeDesc::total_reqs_hit=0
>>>
>>> sun.ci.stubCodeDesc::total_reqs=10335
>>> sun.ci.stubCodeDesc::total_reqs_hit=0
>>>
>>> sun.ci.stubCodeDesc::total_reqs=4698
>>> sun.ci.stubCodeDesc::total_reqs_hit=0
>>>
>>> sun.ci.stubCodeDesc::total_reqs=5059
>>> sun.ci.stubCodeDesc::total_reqs_hit=0
>>>
>>> sun.ci.stubCodeDesc::total_reqs=2763
>>> sun.ci.stubCodeDesc::total_reqs_hit=0
>>>
>>> sun.ci.stubCodeDesc::total_reqs=3035
>>> sun.ci.stubCodeDesc::total_reqs_hit=0
>>>
>>> sun.ci.stubCodeDesc::total_reqs=7244
>>> sun.ci.stubCodeDesc::total_reqs_hit=0
>>>
>>> [4] Box2D external addresses stats:
>>> 5628 0x0000000102601396
>>>    16 0x00000001027cba58
>>>    20 0x00000001027cba60
>>>    85 0x00000001027cc198
>>>   110 0x00000001027d82a2
>>>    55 0x00000001027ee720
>>>   351 0x00000001027eea80
>>>     9 0x0000000102814380
>>>    41 0x00000001028143a0
>>>     2 0x0000000103246460
>>> 3812 0x0000000103246480