RFR JDK-8059510 Compact symbol table layout inside shared archive

[Jiangli Zhou]

On 10/10/2014 04:18 PM, Ioi Lam wrote:
>
> On 10/10/14, 2:06 PM, Jiangli Zhou wrote:
>> Hi Gerard,
>>
>> On 10/10/2014 01:44 PM, Gerard Ziemski wrote:
>>> hi Jiangli,
>>>
>>> On 10/10/2014 3:10 PM, Jiangli Zhou wrote:
>>>> Hi Gerard,
>>>>
>>>> On 10/10/2014 08:12 AM, Gerard Ziemski wrote:
>>>>> hi Jiangli,
>>>>>
>>>>> On 10/9/2014 2:11 PM, Jiangli Zhou wrote:
>>>>>> Hi Gerard,
>>>>>>
>>>>>> Thank you very much for the review. Please see my comments below.
>>>>>>
>>>>>> On 10/09/2014 08:04 AM, Gerard Ziemski wrote:
>>>>>>> hi Jiangli,
>>>>>>>
>>>>>>> I'm a reviewer with small "r" and I'm still going through your 
>>>>>>> code and learning as I go, but so far I have 2 items as my 
>>>>>>> feedback/questions:
>>>>>>>
>>>>>>> #1 Re: "SymbolTable::lookup”
>>>>>>>
>>>>>>>  Symbol* SymbolTable::lookup(int index, const char* name,
>>>>>>>                                int len, unsigned int hash) {
>>>>>>> +  Symbol* s = _shared_table.lookup(name, hash, len);
>>>>>>> +  if (s != NULL) {
>>>>>>> +    return s;
>>>>>>> +  }
>>>>>>> +
>>>>>>>    int count = 0;
>>>>>>>    for (HashtableEntry<Symbol*, mtSymbol>* e = bucket(index); e 
>>>>>>> != NULL; e = e->next()) {
>>>>>>>      count++;  // count all entries in this bucket, not just 
>>>>>>> ones with same hash
>>>>>>>      if (e->hash() == hash) {
>>>>>>>        Symbol* sym = e->literal();
>>>>>>>
>>>>>>> a) Do we need to evaluate the lookup time performance, now that 
>>>>>>> some entries will have to be looked up in 2 separate tables in 
>>>>>>> "SymbolTable::lookup"?
>>>>>>>
>>>>>>> b) Shared table is being looked at 1st, is this the case we expect?
>>>>>>
>>>>>> Those are very good questions. The shared symbol table lookup are 
>>>>>> fast since we can very efficiently locate the specific bucket 
>>>>>> with pre-calculated bucket sizes. The shared table is searched 
>>>>>> first because the symbols contained in that are from archived 
>>>>>> classes, which are the ones used during bootstrap (by default). 
>>>>>> Separating the symbols into two sets do introducing some 
>>>>>> overhead. In this case, I think the effect is negligible.  The 
>>>>>> data from Aleksey's benchmark for classloading showed very small 
>>>>>> difference between the patched and non-patched version.
>>>>>
>>>>> You might be very well right that the performance hit is 
>>>>> negligible, but my point is that you haven't shown that this issue 
>>>>> isn't a problem by backing it up with actual performance data. You 
>>>>> use Aleksey's own benchmark to prove your point, which only came 
>>>>> up during the review and which actually shows the opposite (though 
>>>>> only a slight regression). I would think that we need real 
>>>>> performance data that will prove your assumptions without any doubt.
>>>>
>>>> You have a very good point. I apologize for not providing my 
>>>> first-hand benchmark data. Here are some classloading benchmark 
>>>> results on linux-i586 and linux-arm (soft-float vfp) platforms. 
>>>> 17436 classes were loaded from bootclasspath. For both before and 
>>>> after, the shared archive were used. 10 samples were collected for 
>>>> both before and after.
>>>>
>>>> *Linux ARMv7 tegra board*
>>>> Before(average): 7.9505s
>>>> After(average)   :  7.8601s
>>>>
>>>> *Linux Intel i5*
>>>> Before(average): 1.2162s
>>>> After(average)   : 1.1457s
>>>
>>> This looks promising, but it also looks like a specialized benchmark 
>>> designed to test shared archive behavior. Do we have performance 
>>> regressions numbers from standard benchmarks (ie. refworkload) that 
>>> do not use shared archive path?
>>
>> The test used was designed for benchmarking classloading speed, not 
>> specifically for testing shared archive behavior. Shared archive was 
>> used for both before and after because the shared symbol table would 
>> only be used in that case. The potential performance impact of 
>> looking up the shared symbol table would only manifest in that case. 
>> When class data sharing is not enabled, the shared symbol table is 
>> not used at all.
>>
>> I'll run specjvm with reworkload.
>>
> I remember I ran a bunch of refworkload before and there was no 
> significant difference before/after this change. But I can't seem to 
> find the e-mail now :-(

Here are the spejvm runs on the ARMv7 tegra board. There is no 
measurable lose with the change.

==============================================================================
logs.specjvm.before:
   Benchmark           Samples        Mean     Stdev Geomean Weight
   specjvm98                 8       81.33      1.47
==============================================================================
logs.specjvm.after:
   Benchmark           Samples        Mean     Stdev   %Diff     P 
Significant
   specjvm98                 8       81.72      0.70    0.48 
0.509            *
==============================================================================

Thanks,
Jiangli

>
> - Ioi
>
>
>