Skip to content

Unnecessary synchronization in org.xbill.DNS.Header::getID #215

@maltalex

Description

@maltalex

I'm working on improving the performance of a project that deals with enormous amounts of dns messages. One of the performance bottlenecks I'm seeing is a whole lot of locking in org.xbill.DNS.Header::getID which is implemented like this:

private static final Random random = new SecureRandom();

public int getID() {
  synchronized (random) {
    if (id < 0) {
      id = random.nextInt(0xffff);
    }
    return id;
  }
}

The implementation is not only synchronized - it's synchronized around a static object, meaning that all getID calls in all Header instances are serial! Note that SecureRandom itself is thread-safe and doesn't require locking.

Could the getID implementation be changed to be lock-free using a volatile variable or an AtomicInteger? Or at least switched to use per-instance locking (e.g. synchronize on this or some other non-null field) to reduce the lock's scope?

I'd be happy to submit a PR with a lock-free implementation.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions