Replace use of "get/check/put" with putIfAbsent

316 views Asked by At

I am working with Cassandra and using Datastax Java driver. I am trying to reuse prepared statements by caching it.

  private static final Map<String, PreparedStatement> holder = new ConcurrentHashMap<>();

  public BoundStatement getStatement(String cql) {
    Session session = TestUtils.getInstance().getSession();
    PreparedStatement ps = holder.get(cql);
    // no statement is cached, create one and cache it now.
    if (ps == null) {
      synchronized (this) {
        ps = holder.get(cql);
        if (ps == null) {
          ps = session.prepare(cql);
          holder.put(cql, ps);
        }
      }
    }
    return ps.bind();
  }

My above getStatement method will be called by multiple threads so I have to make sure it is thread safe. I am working with Java 7 so cannot use computeIfAbsent unfortunately.

When I ran my code against static anaylysis tool, it gave me this minor warning which makes me to think is there any better way to write above code in Java 7?

Might be better to replace use of get/check/put with putIfAbsent

Update:

  public BoundStatement getStatement(String cql) {
    Session session = TestUtils.getInstance().getSession();
    PreparedStatement ps = holder.get(cql);
    // no statement is cached, create one and cache it now.
    if (ps == null) {
      ps = session.prepare(cql);
      PreparedStatement old = holder.putIfAbsent(cql, ps);
      if (old!=null)
        ps=old;
    }
    return ps.bind();
  }
1

There are 1 answers

5
Matt Timmermans On BEST ANSWER

It's not too bad the way you have it, except that one thread can block another even if they're not trying to make the same prepared statement.

Using computeIfAbsent in Java 8 would indeed be much better. In Java 7, you can do this:

ps = holder.get(cql);
if (ps == null) {
  ps = session.prepare(cql);
  PreparedStatement old = holder.putIfAbsent(cql, ps);
  if (old!=null)
    ps=old;
}

You will occasionally make an unnecessary PreparedStatement if two threads try to make the same one at the same time, but in the worst case that's just equivalent to not using a cache.

Alternatively, if you can use the guava libraries, then the guava LoadingCache does exactly what you want: https://google.github.io/guava/releases/16.0/api/docs/com/google/common/cache/CacheBuilder.html