maxmind / MaxMind-DB-Reader-java

Java reader for the MaxMind DB format
https://maxmind.github.io/MaxMind-DB-Reader-java/
Apache License 2.0
113 stars 43 forks source link

Huge contention on BufferHolder.get() #65

Closed stashewski-platformio closed 4 years ago

stashewski-platformio commented 4 years ago

Hi everyone.

Using DatabaseReader in highly loaded environment and noticed huge blocking time on this line. If I understand correctly byte buffer is duplicated and only used locally on method's stack memory. Why is it made as synchronized ? Are there any workarounds ?

Thanks!

oschwald commented 4 years ago

duplicate is not documented as thread safe, which is why it is synchronized. Although it seems like something that would probably be safe for most reasonable implementations, there are no guarantees.

I'd be curious to know more about your environment where this is an actual issue. That line should only be called once per lookup request and should be very quick to execute. I am a bit surprised it is causing contention. How many threads does your application have going at once and how many lookups per second are you doing in each of them? How did you determine this was a major source of blocking?

stashewski-platformio commented 4 years ago

Yeap, thats true, my bad.

There are about 1 million requests per minute and on each request we are checking connection type, city etc and around 5k threads. image

Recording was done for one minute and it showed 1.5 day of contention time. As a quick fix we are using several database readers instances and choosing randomly on each request from a "pool". Ideally it would be nice to load it once and just read everything required from memory.

oschwald commented 4 years ago

Wow, that is a lot of threads. One possibility would be to create a pool of byte buffers that get reused. This would both reduce allocations slightly and, if implemented properly, could reduce or eliminate contention.

oschwald commented 4 years ago

1.4.0 has been released with the synchronized removed. Thanks!