RFR (4th): 8023524: Mechanism to dump generated lambda classes / log lambda code generation

[Remi Forax]

On 10/02/2013 01:04 AM, Henry Jen wrote:
> Hi,
>
> Please review the updated webrev at
> http://cr.openjdk.java.net/~henryjen/ccc/8023524/3/webrev/
>
> This update addressed comments from Mandy with following,
>
> - call doPrivileged with specific file permission, "<<ALL FILES>>", "write".
> - Use nio package to write deal with FS, also create directory hierarchy
> with package name instead of flat files.
> - If not specify a value or empty string, the directory is default to
> current working directory as "." is specified.
>
> We continue to use local encoding rule as other suggestions doesn't fit.
> The reason we cannot use something like URLEncoder is that ":" won't get
> encoded and it's not valid character for filename on Windows. All the
> suggestions so far seems to have that same issue.
>
> Cheers,
> Henry
>

Hi Henry,
most of the code that use atomic values is not needed if you do all 
these checks
in the static block (because the static block is protected by a lock) and
also instead of injecting the dump_dir, I think it's better to ask for it
in ProxyClassesDumper.

so first, nitpicking!, DUMP_PATH_PROPERTY doesn't need to be a static field,
the code below is equivalent to yours but avoid to create yet another 
static field.

// For dumping generated classes to disk, for debugging purposes
/* package */ static final String LAMBDA_PROXY_DUMP_DIR;
static {
     String DUMP_PATH_PROPERTY = "jdk.internal.lambda.dumpProxyClasses";
     LAMBDA_PROXY_DUMP_DIR= AccessController.doPrivileged(new GetPropertyAction(DUMP_PATH_PROPERTY));
}


// If requested, dump out to a file for debugging purposes
if (LAMBDA_PROXY_DUMP_DIR  != null) {
            AccessController.doPrivileged(new PrivilegedAction<Void>() {
                @Override
                public Void run() {
                    ProxyClassesDumper dumper = ProxyClassesDumper.getInstance();   // dont send DUMP_DIR here
...

then as I said the code in ProxyClassesDumper can be simplified if instead
of sending the dump directory, the ProxyClassesDumper ask for it.

final class ProxyClassesDumper {
     ...

     private final static ProxyClassesDumper INSTANCE;
     static {
        // ProxyClassesDumper.getInstance() should never be called
        // in the static block of InnerClassLambdaMetafactory
        // because it will create a circular dependency (a NPE)
        String pathname = InnerClassLambdaMetafactory.LAMBDA_PROXY_DUMP_DIR.trim();
        Path dumpDir = Paths.get(pathname.isEmpty() ? "." : pathname);
        dumpDir = validateDumpDir(dumpDir)? dumpDir: null;
        INSTANCE = new ProxyClassesDumper(dumpDir);
     }
  
     private final Path dumpDir;  // null if invalid

     // implement dump singleton instance, that's all we need right now
     public static ProxyClassesDumper getInstance() {
         return INSTANCE;
     }

     private ProxyClassesDumper(String path) {
         this.path = path;
     }

     private static boolean validateDumpDir(Path path) {
         String errMsg;   // no need to initialize errMsg here
         if (!Files.exists(path)) {
     ...
         
     public void dumpClass(String className, final byte[] classBytes) {
         if (dumpDir == null) {
           // invalid dump directory, silently ignore it
           return;
         }
         ...

cheers,
Rémi