RFR(S) 8214450 Use a higher-level function to store Strings into CDS archive

[Ioi Lam]

Hi Jiangli,

Thanks for looking into this. Looks like it's not worth the trouble to 
make this change. I'll withdraw this RFR.

I'll file a separate bug for fixing/commenting the 
check_closed_archive_heap_region_object_class

Thanks
- Ioi

On 11/28/18 11:54 AM, Jiangli Zhou wrote:
> Hi Ioi,
>
> The would change the archiving behavior for the edge cases with extra 
> large String (larger than one GC region). The code in 
> StringTable::create_archived_string() skips a String that's too large 
> to be archive-able and continues without aborting the dump process. 
> Extremely large Strings are rare but possible, we don't need to fail 
> the whole dumping process in that case.
>
> HeapShared::archive_reachable_objects_from is generic subgraph 
> archiving routine, which needs to bail out when an object in the graph 
> fails to archive (because we don't do unrolling). Changing 
> StringTable::create_archived_string to use 
> HeapShared::archive_reachable_objects_from also adds more complexities 
> to subgraph archiving. Now it needs to deal with NULL subgraph info.
>
> We can either leave StringTable::create_archived_string unchanged or 
> create a new API that does graph archiving without the subgraph_info 
> Klass recording for the Strings. The API needs to walk a graph twice. 
> First time is to detect if an object is too large and skip the graph. 
> Or, we need to archive the leaf object first.
>
> The bug fix in 
> HeapShared::check_closed_archive_heap_region_object_class looks good.
>
> Thanks,
> Jiangli
>
> On 11/28/18 10:21 AM, Ioi Lam wrote:
>> http://cr.openjdk.java.net/~iklam/jdk12/8214450-cleanup-string-archive.v01/ 
>>
>> https://bugs.openjdk.java.net/browse/JDK-8214450
>>
>> This is a clean up that simplifies the archiving of shared strings. 
>> Also:
>>    - removed java_lang_String::set_value_raw which has become dead code.
>>    - fixed a bug in 
>> HeapShared::check_closed_archive_heap_region_object_class,
>>      and renamed/commented this function for clarity.
>>
>> All CDS tests passed locally. Running hs-tiers{1-4} just for sanity.
>