RFR: 8301992: Embed SymbolTable CHT node [v4]

[Calvin Cheung]

On Thu, 16 Feb 2023 18:53:17 GMT, Ioi Lam <iklam at openjdk.org> wrote:

>> Calvin Cheung has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   add Afree() for permanent symbols
>
> src/hotspot/share/classfile/symbolTable.cpp line 168:
> 
>> 166:       if (!SymbolTable::arena()->Afree(memory, alloc_size)) {
>> 167:         log_trace_symboltable_helper(&value, "Leaked permanent symbol");
>> 168:       }
> 
> We should avoid freeing if DumpSharedSpaces is true, so that the code is symmetrical with allocate_node_impl(). Maybe this:
> 
> 
> #if INCLUDE_CDS
>   if (DumpSharedSpaces) {
>     // no deallocation is needed
>   } else
> #endif
>   if (value.refcount() != PERM_REFCOUNT) {

How about performing the above check at the beginning of the function, right after the assert statement as follows?
Let me know if the added assert is necessary.

--- a/src/hotspot/share/classfile/symbolTable.cpp
+++ b/src/hotspot/share/classfile/symbolTable.cpp
@@ -151,6 +151,13 @@ public:
     // If #2, then the symbol is dead (refcount==0)
     assert(value.is_permanent() || (value.refcount() == 1) || (value.refcount() == 0),
            "refcount %d", value.refcount());
+#if INCLUDE_CDS
+    if (DumpSharedSpaces) {
+      assert(value.is_permanent(), "symbol should be permanent during CDS dumping");
+      // no deallocation is needed
+      return;
+    }
+#endif
     if (value.refcount() == 1) {
       value.decrement_refcount();
       assert(value.refcount() == 0, "expected dead symbol");

-------------

PR: https://git.openjdk.org/jdk/pull/12562