Increment Value of in a Hashmap in a thread safe manner keeping high performance without synchronized?

497 views Asked by At

I have a Model which has different Variables.

public class Model implements Serializable{

    public final static int STATE_INIT = 0;
    public final static int STATE_READY = 1;

    private Integer state = STATE_INIT;
    private HashMap<Integer,Integer>pageRequests = new HashMap<>();
    private HashMap<Integer,Integer>impr = new HashMap<>();
    private HashMap<Integer,Integer>clicks = new HashMap<>();

    public void incrementPageRequests(int accountId){

   if(this.pageRequests.get(accountId) != null){
       this.pageRequests.put(accountId,this.pageRequests.get(accountId) +1);
   } else {
       this.pageRequests.put(accountId,1);
   }
}

public void incrementImprServed(int accountId){

    if(this.imprServed.get(accountId) != null){
        this.imprServed.put(accountId,this.imprServed.get(accountId) +1);
    } else {
        this.imprServed.put(accountId,1);
    }
}

public void incrementClicksServed(int accountId){

    if(this.clicksServed.get(accountId) != null){
        this.clicksServed.put(accountId,this.clicksServed.get(accountId) +1);
    } else {
        this.clicksServed.put(accountId,1);
    }
}

}

Now when i start the server the model is created it is a singleton bean. i want to be able to modify the hashmap of the model when some one calls the endpoint

/increment

@GetMapping(path = "/increment")
    public String increment(){
        model.incrementPageRequests(1);
        return "Okay";
    }

Presently this incrementPageRequest is not thread safe when i add a synchronized key word the method becomes thread safe , but i have heard that synchronized is very costly and i am looking for high throughput and performance .

How can i acheive the same without synchronized and keeping high performance?

Update

Tried with Concurrent HashMap and still it fails i am using Jmeter for testing concurrent calls to the api

How do i change this logic so that it works in concurrent hashmap

 if(this.pageRequests.get(accountId) != null){
           this.pageRequests.put(accountId,this.pageRequests.get(accountId) +1);
       } else {
           System.out.println("Here");
           this.pageRequests.putIfAbsent(accountId,1);
       }
1

There are 1 answers

2
Oleg Kovalov On

At first: create a benchmark, before deciding what of the solutions helps you.

Also you're doing a bit redundant work here(and in other methods too):

if(this.pageRequests.get(accountId) != null){
    this.pageRequests.put(accountId,this.pageRequests.get(accountId) +1);
} else {
    this.pageRequests.put(accountId,1);
}

Instead

final String value = this.pageRequests.get(accountId);
if(value == null){
    this.pageRequests.put(accountId, 1);
    return;
}
this.pageRequests.put(accountId, value + 1);

Now you'll have 1 read access to the map less.

Regarding your second question "How do i change this logic so that it works in concurrent hashmap" change this:

private HashMap<Integer, Integer> pageRequests = new HashMap<>();

too:

private Map<Integer, Integer> pageRequests = new ConcurrentHashMap<>();

Keeping private field as interface allows you simpler change implementation of the map.