Re: RFR 8071962 The SA code needs to be updated to support Symbol lookup from the shared archive

[serguei.spit...@oracle.com]

Jiangli,

The fix looks good.
One minor comment/question though.

It looks like the comment needs an update as it does not return null anymore
if the string has not been found in the symbol table.

agent/src/share/classes/sun/jvm/hotspot/memory/SymbolTable.java

@@ -74,20 +81,23 @@
   /** Clone of VM's "temporary" probe routine, as the SA currently
       does not support mutation so lookup() would have no effect
       anyway. Returns null if the given string is not in the symbol
       table. */
   public Symbol probe(byte[] name) {
+    Symbol sym;
     long hashValue = hashSymbol(name);
     for (HashtableEntry e = (HashtableEntry) bucket(hashToIndex(hashValue)); e 
!= null; e = (HashtableEntry) e.next()) {
       if (e.hash() == hashValue) {
-         Symbol sym = Symbol.create(e.literalValue());
+         sym = Symbol.create(e.literalValue());
          if (sym.equals(name)) {
            return sym;
          }
       }
     }
-    return null;
+
+    sym = sharedTable.probe(name, hashValue);
+    return sym;
   }



You may also want to update the copyright comments.


Thanks,
Serguei


On 1/29/15 5:46 PM, Jiangli Zhou wrote:

Hi Serguei,

Thanks for noticing that. It’s actually a duplicated bug ID for the same issue. I just fixed the web rev: http://cr.openjdk.java.net/~jiangli/8071962/webrev.00/ <http://cr.openjdk.java.net/%7Ejiangli/8071962/webrev.00/>.

Thanks,
Jiangli

On Jan 29, 2015, at 5:40 PM, serguei.spit...@oracle.com <mailto:serguei.spit...@oracle.com> wrote:

On 1/29/15 5:13 PM, Jiangli Zhou wrote:

Hi,

Please review the change for JDK-8071962 <https://bugs.openjdk.java.net/browse/JDK-8071962>, "The SA code needs to be updated to support Symbol lookup from the shared archive”.

In JDK-8059510 <https://bugs.openjdk.java.net/browse/JDK-8059510>, we introduced a compact form of hash table for the symbols in shared archive. The shared symbols are stored separately from the regular symbol table. The VM looks up both tables when searching for existing symbol at runtime. The SA code needs to do the same when looking up symbols.

Webrev: http://cr.openjdk.java.net/~jiangli/8067977/webrev.00/ <http://cr.openjdk.java.net/%7Ejiangli/8067977/webrev.00/>

Jiangli,

I'm not reviewing, just wanted to make sure there is no typo here ...

The webrev bug number is 8067977, but the RFR bug number is 8071962 which is strange.

Thanks,
Serguei

Tested on both 32-bit and 64-bit platforms:
bin/java sun.jvm.hotspot.tools.DumpJFR <pid>

JPRT tests in progress.

Thanks,
Jiangli