String.intern() and concurrency

Have there been times when you needed some form of locking but could not find an easy way to granularize it? For example lets say you have

Map<Person, Double> salariesMap = Collections.synchronizedMap(new HashMap<Person, Double>());//BigDecimal might be a better value type

void giveRaise(final Person person, double raisePercentage){
    synchronized(salariesMap){
        oldSalary = salariesMap.get(person);
        Double newSalary = (1 + raisePercentage/100.0) * oldSalary;
        salariesMap.put(person, newSalary);
    }
}

Our first attempt at making the locking granular could be:

void giveRaise(final Person person, double raisePercentage){
    synchronized(person){
        Double oldSalary = salariesMap.get(person);
        Double newSalary = (1 + raisePercentage/100.0) * oldSalary;
        salariesMap.put(person, newSalary);
    }
}

Unless there is a guarantee that for any two Person instances p1 and p2, p1.equals(p2) implies p1 == p2, our locking on the Person object (the map key) would be wrong.

Such a guarantee can be obtained if all Person instances are looked up from a pool of unique Persons.

But then again what if an instance was obtained from the pool and a reference to it held long after it was removed from the pool? To avoid such a stale reference, would one now lock the pool itself too?

Map<String, Person> persons = Collections.synchronizedMap(new HashMap<String, Person>());

void giveRaise(String pName, double raisePercentage){
    synchronized(persons){
        final Person p = persons.get(pName);
        synchronized(p) {
            Double oldSalary = salariesMap.get(p);
            Double newSalary = (1 + raisePercentage/100.0) * oldSalary;
            salariesMap.put(person, newSalary);
        }
    }
}

But this beats the whole purpose of locking only on one Person key for granularity!

Luckily such a unique pool is maintained by the JVM itself for strings and the unique instances in the pool can be accessed via String.intern()

Thus we can obtain a granular locking as follows:

void giveRaise(final Person p, double raisePercentage){
    synchronized(p.getName().intern()){
        Double oldSalary = salariesMap.get(p);
        Double newSalary = (1 + raisePercentage/100.0) * oldSalary;
        salariesMap.put(p, newSalary);
    }
}

Note that the use of a ConcurrentMap instead of Collections.synchronizedMap in the above might lead to better parallelism in the calls to put above. Also, we could probably use replace() instead of simple put():

ConcurrentMap<Person, Double> salariesMap = new ConcurrentHashMap<Person, Double>();

void giveRaise(final Person person, double raisePercentage){
    synchronized(person.getName().intern()){
        oldSalary = salariesMap.get(person);
        Double newSalary = (1 + raisePercentage/100.0) * oldSalary;
        if(!salariesMap.replace(person, oldSalary, newSalary)) {
            logger.error("Salary was modified concurrently");
        }
    }
}

Also see comments on http://stackoverflow.com/questions/348985/deadlock-on-synchronized-string-intern

If the "key" is likely to collide with the key of another class of objects then synchronize(key.intern()) will introduce an unnecessary mutual exclusion among completely unrelated code. In such a case one might instead use a WeakHashMap<String, WeakReference<Object>> as a mapping from a string to a lock object. Then by having one such map for each class of objects, the accidental mutual exclusion because of shared keys can be avoided.

Using a custom classloader

So, recently I was using a code generation tool, and needed to invoke the code generation multiple times on the same input files but pass in different configuration settings each time. But I was disappointed when the second (and subsequent) invocations would not generate any output files. After downloading the source code for the tool and spending some time debugging, I discovered that there was a private static Set<File> files field in a class which would keep track of the files it had already handled.
Now to get around this there were two solutions:

1. After each invocation of code generation, clear the set using reflection
2. Use a different classloader to load the code generator each time

Now for this particular scenario, it was easier to simply use reflection. However, there can be cases when you want to concurrently use multiple instances of some class but avoid cross-talk, e.g. due to static fields for instance. A custom classloader can be used in such cases. You will have to resort to reflection for making the actual method invocations though.

Example:

package com.usta;

public class Service {
    private static final Service INSTANCE = new Service();

    public static Service getInstance() { return INSTANCE; }

    //private constructor
    private Service() {}

    public void doIt(String arg) {
        System.out.println("Hey " + arg + " " + hashCode());
    }
}

package com.usta;

import java.io.DataInputStream;
import java.io.File;
import java.io.FileInputStream;
import java.io.FileNotFoundException;
import java.io.FilenameFilter;
import java.io.IOException;
import java.io.InputStream;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
import java.util.jar.JarEntry;
import java.util.jar.JarFile;

public class CustomClassLoader extends ClassLoader {
    private final File                      classpathDir;
    private ConcurrentMap<String, Class<?>> classes = new ConcurrentHashMap<String, Class<?>>();

    public CustomClassLoader(String directory) {
        super();
        this.classpathDir = new File(directory);
        if (!classpathDir.isDirectory())
            throw new RuntimeException("Not a folder " + directory);
    }

    @Override
    public Class<?> loadClass(String name) throws ClassNotFoundException {
        return name.startsWith("com.usta") ? findClass(name) : findSystemClass(name);
    }

    // TODO support inner classes
    private String binaryNameToFileName(String binaryName) {
        return binaryName.replaceAll("\\.", "/") + ".class";
    }

    @Override
    protected Class<?> findClass(String clazz) throws ClassNotFoundException {
        if (classes.containsKey(clazz)) {
            return classes.get(clazz);
        }
        synchronized (clazz.intern()) {
            // we might have already defined the class on a separate thread
            if (classes.containsKey(clazz)) {
                return classes.get(clazz);
            }
            InputStream is = null;
            int size = 0;
            for (File f : classpathDir.listFiles(new FilenameFilter() {
                public boolean accept(File arg0, String arg1) {
                    return arg1.endsWith(".jar");
                }
            })) {
                try {
                    JarFile jarFile = new JarFile(f);
                    JarEntry clazzEntry = jarFile.getJarEntry(binaryNameToFileName(clazz));
                    if (clazzEntry != null) {
                        is = jarFile.getInputStream(clazzEntry);
                        size = (int) clazzEntry.getSize();
                        break;
                    }
                } catch (IOException e) {
                    throw new ClassNotFoundException(e.getMessage());
                }
            }
            if (is == null) {
                String file = classpathDir.getPath() + System.getProperty("file.separator")
                        + binaryNameToFileName(clazz);
                File classFile = new File(file);
                if (!classFile.exists() || !classFile.isFile())
                    return super.findClass(clazz);
                size = (int) classFile.length();

                try {
                    is = new FileInputStream(classFile);
                } catch (FileNotFoundException e) {
                    throw new ClassNotFoundException(e.getMessage());
                }
            }

            DataInputStream dis = new DataInputStream(is);
            byte buff[] = new byte[size];
            try {
                dis.readFully(buff);
                dis.close();
            } catch (IOException e) {
                throw new ClassNotFoundException(e.getMessage());
            }

            classes.putIfAbsent(clazz, super.defineClass(clazz, buff, 0, size));
            return classes.get(clazz);
        }
    }
}

Now the use of this custom class loader to load multiple instances of Service would be:

Service service = Service.getInstance();
service.doIt("Buddy");
ClassLoader loader = new CustomClassLoader("/users/ustamansangat/workspace/test/target");
Class<?> serviceClass = loader.loadClass("com.usta.Service");
Object service2 = serviceClass.getMethod("getInstance").invoke(null);
serviceClass.getMethod("doIt", String.class).invoke(service2, "Dude");

Constructors v Init blocks

With time code begins to bloat and the procrastination regarding re-factoring and code clean-up makes a new comer to the project feel like groping her way out of a maze.

One tiny contributor to such lack of maintenance is how some classes have multiple non-trivial constructors and modifications in one constructor of a class is forgotten to be made in other constructors.  Two ways to avoid this:

1. Things that can be done in an initialization block should be done there instead of a constructor. 
2. If the language supports it, constructor chaining should be utilized to avoid duplication of  code.

By constructor chaining, I mean something like the following (Java):

class Person {
    private final String name;
    private int salary;

    public Person(String name) {
         this.name = name == null || name.length()== 0 ? "anonymous" : name;
    }
    public Person(String name, int salary) {
         this(name);
         this.salary = salary;
    }
}

If you are like me in that you want to declare as many fields final as possible but a field has a non-trivial initialization step, that can also go in an init block.