Search code examples
javadictionarycountdownlatchnull-check

Why the overall performance of application degrades if I do null checks on the maps?


Below is my class which uses CountDownLatch to make sure reads are not happening on the primary, secondary and tertiary maps for the first time whenever writes are happening on those maps.

public class ClientData {

    public static class Mappings {
        public final Map<String, Map<Integer, String>> primary;
        public final Map<String, Map<Integer, String>> secondary;
        public final Map<String, Map<Integer, String>> tertiary;

        public Mappings(
            Map<String, Map<Integer, String>> primary,
            Map<String, Map<Integer, String>> secondary,
            Map<String, Map<Integer, String>> tertiary
        ) {
            this.primary = primary;
            this.secondary = secondary;
            this.tertiary = tertiary;
        }
    }

    private static final AtomicReference<Mappings> mappings = new AtomicReference<>();
    private static final CountDownLatch hasBeenInitialized = new CountDownLatch(1);

    public static Mappings getMappings() {
        try {
            hasBeenInitialized.await();
            return mappings.get();
        } catch (InterruptedException e) {
            Thread.currentThread().interrupt();
            throw new IllegalStateException(e);
        }
    }

    public static void setMappings(
        Map<String, Map<Integer, String>> primary,
        Map<String, Map<Integer, String>> secondary,
        Map<String, Map<Integer, String>> tertiary
    ) {
        setMappings(new Mappings(primary, secondary, tertiary));
    }

    public static void setMappings(Mappings newMappings) {
        mappings.set(newMappings);
        hasBeenInitialized.countDown();
    }
}

And below is my background thread class which is only responsible for setting all the three maps (look for parseResponse method below). It runs every 10 minutes.

public class TempBackgroundThread {

    // parse the response and store it in a variable
    private void parseResponse(String response) {
        //...       
        Map<String, Map<Integer, String>> primaryTables = null;
        Map<String, Map<Integer, String>> secondaryTables = null;
        Map<String, Map<Integer, String>> tertiaryTables = null;

        //...

        // store the three map data in ClientData class variables if anything has changed  
        // which can be used by other threads, this will be updated once every four or five months
        if(changed) {
            ClientData.setMappings(primaryTables, secondaryTables, tertiaryTables);
        }
    }
}

Problem Statement:

If I am doing all sort of null or sanity checks on my mappings object and primary, secondary and tertiary maps, peformance degrades a lot (not sure why). But if I don't do any sanity or null checks, performance comes very good. Can anyone explain me what's wrong and why it happens?

Below is an example -

I am using ClientData class to get all the mappings in my main thread. As you can see below, I am doing all sort of sanity checks to make sure mappings, mappings.primary, mappings.secondary and mappings.tertiary are not empty. If they are empty, then log an error and return

class Task implements Callable<String> {

    public Task() {
    }

    public String call() throws Exception {

        int compId = 100;
        String localPath = "hello";
        String remotePath = "world";

        Mappings mappings = ClientData.getMappings(); 

        if (MyUtilityClass.isEmpty(mappings)
                || (MyUtilityClass.isEmpty(mappings.primary) && MyUtilityClass
                        .isEmpty(mappings.secondary))
                || MyUtilityClass.isEmpty(mappings.tertiary)) {

            // log error and return
        }

        // otherwise extract values from them
        String localPAddress = null;
        String remotePAddress = null;
        if (MyUtilityClass.isNotEmpty(mappings.primary)) {
            String localPId = mappings.primary.get(localPath).get(compId);
            localPAddress = mappings.tertiary.get(localPath).get(
                    Integer.parseInt(localPId));
            String remotePId = mappings.primary.get(remotePath).get(compId);
            remotePAddress = mappings.tertiary.get(remotePath).get(
                    Integer.parseInt(remotePId));
        }

        String localSAddress = null;
        String remoteSAddress = null;
        if (MyUtilityClass.isNotEmpty(mappings.secondary)) {
            String localSId = mappings.secondary.get(localPath).get(compId);
            localSAddress = mappings.tertiary.get(localPath).get(
                    Integer.parseInt(localSId));
            String remoteSId = mappings.secondary.get(remotePath).get(compId);
            remoteSAddress = mappings.tertiary.get(remotePath).get(
                    Integer.parseInt(remoteSId));
        }

        // now use - localPAddress, remotePAddress, localSAddress and remoteSAddress
    }
}

With the above sanity and null checks on primary, secondary and tertiary mappings, overall performance (95th percentile) of application comes as 4 ms.

But if I do it like this without any sanity checks or null checks on primary, secondary and tertiary mappings, I get overall perforamnce (95th percentile) as 0.87 ms.

class Task implements Callable<String> {

    public Task() {
    }

    public String call() throws Exception {

        int compId = 100;
        String localPath = "hello";
        String remotePath = "world";

        Mappings mappings = ClientData.getMappings(); 

        String localPId = mappings.primary.get(localPath).get(compId);
        String localPAddress = mappings.tertiary.get(localPath).get(Integer.parseInt(localPId));
        String remotePId = mappings.primary.get(remotePath).get(compId);
        String remotePAddress = mappings.tertiary.get(remotePath).get(Integer.parseInt(remotePId));
        String localSId = mappings.secondary.get(localPath).get(compId);
        String localSAddress = mappings.tertiary.get(localPath).get(Integer.parseInt(localSId));
        String remoteSId = mappings.secondary.get(remotePath).get(compId);
        String remoteSAddress = mappings.tertiary.get(remotePath).get(Integer.parseInt(remoteSId));

        // now use - localPAddress, remotePAddress, localSAddress and remoteSAddress
    }
}

Below is my isEmpty and isNotEmpty method -

public static boolean isNotEmpty(Object obj) {
    return !isEmpty(obj);
}

public static boolean isEmpty(Object obj) {
    if (obj == null)
        return true;
    if (obj instanceof Collection)
        return ((Collection<?>) obj).size() == 0;

    final String s = String.valueOf(obj).trim();

    return s.length() == 0 || s.equalsIgnoreCase("null");
}

Solution

  • See how often your code gets to this point. This can be expensive with some complex objects and their heavy #toString() methods:

    final String s = String.valueOf(obj).trim();
    

    Also it creates temporary garbage that might cause Garbage Collection while your test is counting.