0

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);
       }
INFOSYS
  • 1,465
  • 9
  • 23
  • 50
  • Synchronization is mot 'very costly'. If you think it will be a bottleneck you need to acquire some evidence. `ConcurrentHashMap` does not 'fail', unless you have coding bugs. Unclear what you're asking. – user207421 Jun 17 '17 at 09:32
  • You think `synchronized` is going to be the *slow* section in this REST call? really? – Nim Jun 17 '17 at 09:36
  • I am not sure about it buti tried changing hashmap to concurrent hashmap but it doesn't work. Can you please let me waht i need yo change in my method? @EJP – INFOSYS Jun 17 '17 at 09:45
  • @NIM i think you so. I ak not sure will it not affect the performance . That is the reason i asked the question – INFOSYS Jun 17 '17 at 09:46
  • No,`synchronized` is the least of your concerns (esp if all you are doing is incrementing a value in a hash map!) Unless you will have millions of calls to this endpoint (at which point you will have other problems to deal with..) – Nim Jun 17 '17 at 10:01
  • @Nim the hashmap key might change its one in this case but i can chnage and then increment , i was wondering how concurrent hashmap can help can you please let me know as i will have illion hits on this endpoint for sure – INFOSYS Jun 17 '17 at 10:06

1 Answers1

0

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.

Oleg Kovalov
  • 734
  • 11
  • 33
  • it throws a null pointer execption as on startup the pagerequesthashmap is empty even try catch no solving the issue – INFOSYS Jun 17 '17 at 13:21
  • Even changing it to Integer and then running if i am firing 500 concurrent request the increment value is 434 where as it should be 500 – INFOSYS Jun 17 '17 at 13:24