From 8d7b0ac425da17a5491513f30fe8bd65c126db8f Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Fri, 12 Jun 2020 09:50:48 -0700 Subject: [PATCH 1/2] Remove use of synchronized around duplicate() Closes #65, #69, and #72. --- CHANGELOG.md | 2 ++ src/main/java/com/maxmind/db/BufferHolder.java | 17 ++++++++++++++++- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6c4e2aa6..3d669965 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,8 @@ 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. 1.3.1 (2020-03-03) ------------------ 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(); } } From 39d063126df8db2041194856014742ba570cc784 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Fri, 12 Jun 2020 09:53:23 -0700 Subject: [PATCH 2/2] Upgrade Jackson Databind --- CHANGELOG.md | 1 + pom.xml | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3d669965..188f7feb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ CHANGELOG #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