diff --git a/CHANGELOG.md b/CHANGELOG.md index 6c4e2aa6..188f7feb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,9 @@ CHANGELOG control byte in the data section rather than an `ArrayIndexOutOfBoundsException`. Reported by Edwin Delgado H. GitHub #68. +* In order to improve performance when lookups are done from multiple + threads, a use of `synchronized` has been removed. GitHub #65 & #69. +* `jackson-databind` has been upgraded to 2.11.0. 1.3.1 (2020-03-03) ------------------ diff --git a/pom.xml b/pom.xml index 2ca25551..d9764a2d 100644 --- a/pom.xml +++ b/pom.xml @@ -45,7 +45,7 @@ com.fasterxml.jackson.core jackson-databind - 2.10.3 + 2.11.0 diff --git a/src/main/java/com/maxmind/db/BufferHolder.java b/src/main/java/com/maxmind/db/BufferHolder.java index fb781aff..856bebcd 100644 --- a/src/main/java/com/maxmind/db/BufferHolder.java +++ b/src/main/java/com/maxmind/db/BufferHolder.java @@ -58,7 +58,22 @@ final class BufferHolder { * Returns a duplicate of the underlying ByteBuffer. The returned ByteBuffer * should not be shared between threads. */ - synchronized ByteBuffer get() { + ByteBuffer get() { + // The Java API docs for buffer state: + // + // Buffers are not safe for use by multiple concurrent threads. If a buffer is to be used by more than + // one thread then access to the buffer should be controlled by appropriate synchronization. + // + // As such, you may think that this should be synchronized. This used to be the case, but we had several + // complaints about the synchronization causing contention, e.g.: + // + // * https://github.com/maxmind/MaxMind-DB-Reader-java/issues/65 + // * https://github.com/maxmind/MaxMind-DB-Reader-java/pull/69 + // + // Given that we are not modifying the original ByteBuffer in any way and all currently known and most + // reasonably imaginable implementations of duplicate() only do read operations on the original buffer object, + // the risk of not synchronizing this call seems relatively low and worth taking for the performance benefit + // when lookups are being done from many threads. return this.buffer.duplicate(); } }