0

I have a servlet that share the access to a Map< String, InnerObject> , with InnerObject being declared as : 

   public class InnerObject {
        volatile EnumObject obj; //declared volatile because can be accessed by different threads of the servlet

        //.......(Methods that change the status of the obj
   }

and the servlet declared as:

     public class TestServlet {
              Map<String, InnerObject> map;

               public void doGet(..) {
                    //change the value of the map and the inner objects
               }
     }

When I try to access the value of the InnerObjects, I get a slow answer from the servlet. If I change the map to "Volatile" too:

        volatile Map<String, InnerObject> map;

, I get faster answers from the http request.

Is volatile needed in this case? Why is it slower when the volatile is defined only in the internal object?

pokeRex110
  • 833
  • 2
  • 14
  • 26
  • 3
    It all depends on what **exactly** is hidden behind "change the value of the map and the inner objects". And frankly, I doubt making a variable volatile can make any significant change to the time needed to execute a HTTP request: the order of magnitude needed for the IO is much much higher than the order of magnitude needed to read / write a volatile variable. – JB Nizet Aug 25 '15 at 12:02
  • 2
    There is no way you're getting a either slower or faster web response due to the overheads of `volatile`. On the other hand, I suspect thread-unsafe code here due to the misunderstanding of the semantics of `volatile` and actual thread safety requirements. – Marko Topolnik Aug 25 '15 at 12:02
  • I have an application that, with the former solution, timeouts with 10 seconds of http connection and the later one that doesnt fail with 1 second of timeout. I can give you numbers too. I am collecting them meanwhile – pokeRex110 Aug 25 '15 at 12:04
  • 2
    You've got a data race resulting in _incorrect_ code, not _slower_ code. – Marko Topolnik Aug 25 '15 at 12:04
  • 1
    The volatile keyword on an Object is only able to ensure that the reference (pointer) is shared correctly between threads. With primitives the value is shared. You do not need volatile in this case. You should however consider using ConcurrentHashMap; – bhspencer Aug 25 '15 at 12:04
  • How can I have an incorrect code with volatile only? How can I have a race condition with it? – pokeRex110 Aug 25 '15 at 12:06
  • 2
    `volatile` is not enough to make your code thread-safe. You mutate the map concurrently. – Marko Topolnik Aug 25 '15 at 12:07
  • It might be true, but in the logic of writing to the map, I dont have concurrent problems, I call it sequentially. The problem is changing the value of the InnerObject, but I agree with you with the logic – pokeRex110 Aug 25 '15 at 12:10
  • @JBNizet - :) *the order of magnitude needed for the IO is much much higher than the order of magnitude needed to read / write a volatile variable* - From java 7+ JIT optimizations like escape analysis can actually remove unnecessary memory barriers. So, yes, the problem is not with the use of `volatile` – TheLostMind Aug 25 '15 at 12:12
  • You can hardly expect someone to pinpoint the line causing a concurrency issue in the code you haven't shared. But based on the information you did provide so far, my educated guess is a data race (a term distinct from "race condition"). – Marko Topolnik Aug 25 '15 at 12:13
  • _faster_ and _slower_ are more philosophical terms. We are engineers. Show some measured numbers. – Dragan Bozanovic Aug 25 '15 at 12:13
  • @MarkoTopolnik - data race? – TheLostMind Aug 25 '15 at 12:13
  • http://stackoverflow.com/questions/11276259/are-data-races-and-race-condition-actually-the-same-thing-in-context-of-conc – bhspencer Aug 25 '15 at 12:15
  • @MarkoTopolnik I didnt know the term data race, I thought you were referring to the race condition, thanks for mentioning it. You are right about the access. I will provide you with numbers as long as I am done with collecting the statistics – pokeRex110 Aug 25 '15 at 12:16
  • 2
    @pokeRex110: you seem to be concerned about performance. That shouldn't be your concern. Your concern should be correctness. You're mutating a non-thread-safe map and non-thread-safe objects from several threads. That can lead to any result: correct, incorrect, exceptions, data corruption, etc. What you should post is the code. Not performance statistics. Applying volatile randomly is not a suitable solution. – JB Nizet Aug 25 '15 at 12:21
  • 1
    Exactly. You think you've got a performance issue, but your code actually stalls in an infinite loop, deadlock, or similar. That's a _correctness_ issue. – Marko Topolnik Aug 25 '15 at 12:22
  • @JB Nizet you are right. I just changed that. Actually the map is written only once (although theoretically can be written more). I was just interested why does it slow down that much. Thank you for the answers – pokeRex110 Aug 25 '15 at 12:24

0 Answers0