Will JIT optimize this code? Is synchronization required?

Below is a class that holds a map of misspelled to correctly spelled terms. The map is updated periodically by a quartz job by calling updateCache(). Method updatecache processes key and values in the input map and stores them in a temporary map object. After the processing is complete (after for loop), it assigns the temporary map to local class variable misspelledToCorrectlySpelled.

package com.test;

import java.util.HashMap; import java.util.Map;

import org.checkthread.annotations.ThreadSafe;

@ThreadSafe public class SpellCorrectListCacheManager {

private Map<String, String> misspelledToCorrectlySpelled = 
    new HashMap<String, String>(0);

/*
 * invoked by a quartz job thread
 */
public void updateCache(Map<String, String> map) {

    Map<String, String> tempMap = new HashMap<String, String>(map.size());

    for (Map.Entry<String, String> entry : map.entrySet()) {
         //process key and values
        String key = entry.getKey().toLowerCase();
        String value = entry.getValue().toLowerCase();

        tempMap.put(key, value);
    }

    // update local variable
    this.misspelledToCorrectlySpelled = tempMap;
}

/*
 * Could be invoked by *multiple* threads
 */
public Map<String, String> getMisspelledToCorrectlySpelled() {
    return misspelledToCorrectlySpelled;
}

}

Question 1: Will JIT optimize optimize this code?

Actual Code

/*
     * since tempMap is assigned to misspelledToCorrectlySpelled and not
     * used anywhere else, will JIT remove tempMap as shown in the optimized
     * version below?
     */
    Map<String, String> tempMap = new HashMap<String, String>(map.size());

    for (Map.Entry<String, String> entry : map.entrySet()) {
        // process key and values
        String key = entry.getKey().toLowerCase();
        String value = entry.getValue().toLowerCase();

        tempMap.put(key, value);
    }

    this.misspelledToCorrectlySpelled = tempMap;

Optmized Code

this.misspelledToCorrectlySpelled = new HashMap<String, String>(map.size());

    for (Map.Entry<String, String> entry : map.entrySet()) {
         //process key and values
        String key = entry.getKey().toLowerCase();
        String value = entry.getValue().toLowerCase();

        this.misspelledToCorrectlySpelled.put(key, value);
    }

Question开发者_运维技巧 2: Assuming JIT won't optimize the code, should method getMisspelledToCorrectlySpelled be synchronized?

/*
     * is this assignment atomic operation?
     * 
     * Does this needs to be synchronized? 
     * 
     * By not synchronizing, the new map may not 
     * be visible to other threads *immediately* -- this is 
     * ok since the new map will be visible after a bit of time 
     * 
     */
    this.misspelledToCorrectlySpelled = tempMap;
}

---

You should use an AtomicReference to store the new map, in order to avoid synchronization and visibility problems. But the biggest problem in your code is that you give access to a non-thread-safe mutable map to several threads. You should wrap your map into an unmodifiable map :

private AtomicReference<Map<String, String>> misspelledToCorrectlySpelled = 
    new AtomicReference<Map<String, String>>(Collections.unmodifiableMap(new HashMap<String, String>(0)));

/*
 * invoked by a quartz job thread
 */
public void updateCache(Map<String, String> map) {

    Map<String, String> tempMap = new HashMap<String, String>(map.size());

    for (Map.Entry<String, String> entry : map.entrySet()) {
         //process key and values
        String key = entry.getKey().toLowerCase();
        String value = entry.getValue().toLowerCase();

        tempMap.put(key, value);
    }

    // update local variable
    this.misspelledToCorrectlySpelled.set(Collections.unmodifiableMap(tempMap));
}

/*
 * Could be invoked by *multiple* threads
 */
public Map<String, String> getMisspelledToCorrectlySpelled() {
    return misspelledToCorrectlySpelled.get();
}

To answer your question about JIT optimization : no, the JIT won't remove the temp map usage.

---

Synchronization is required where synchronization is required. Nothing about the JIT can be assumed except that a compliant implementation will be compliant with the JLS and Java Memory Model and will honor the code designed with these rules. (There are multiple methods of synchronization, not all utilize the synchronized keyword.)

Synchronization is required here unless it's "okay" that a stale version is seen. (That is likely not the case here, and it could be a very stale version with caches and all -- so not something to bet on!). The "assignment of the reference" itself is atomic insofar as no "partial write" can occur, but it is not guaranteed to be [immediately] propagated ("visible") across all threads.

Happy coding.