Details
-
Improvement
-
Status: Closed
-
Major
-
Resolution: Fixed
-
3.3.0
-
None
-
Patch
Description
This issue references the following mailing list post:
http://mail-archives.apache.org/mod_mbox/marmotta-users/201511.mbox/%3C87pozjwzp4.fsf%40dishevelled.net%3E
I've been doing some performance tuning work with Tom Johnson and Mark Breedlove of the DPLA (who have both posted recently), and have noticed a possible improvement for performance when multiple writers are PUTing records concurrently.
While running a benchmark where multiple workers PUT records via Marmotta's LDP interface, I noticed that all but one of the threads were waiting on a lock inside the JVM. The code in question was this `synchronized` block in KiWiValueFactory.createStatement:
https://github.com/dpla/marmotta/blob/master/libraries/kiwi/kiwi-triplestore/ src/main/java/org/apache/marmotta/kiwi/sail/KiWiValueFactory.java#L567
This code takes a lock on the `registry` object (shared by all connections), and holds it while calling connection.getTripleId(). That query is generally pretty fast, but it's called frequently enough that the cost of the locking can still be quite high.
I ran a test where 5 concurrent writers each PUT random records via Marmotta's LDP interface. I ran 6 test rounds, with each round adding an extra millisecond delay to the getTripleId() call (simulating between 0 and 5 milliseconds of network/database latency). As that query gets slower, the effect of the Java locking becomes more dramatic, as you can see from the response times here:
http://dishevelled.net/marmotta-coarse-locking-all-responses.png
With no added delay (and Postgres running on the same machine as Marmotta), most PUT requests complete within about 500ms. With 5 milliseconds of delay, the same PUT requests take between 2.5 and 3 seconds.
I reworked the code to reduce the time that lock was held, and reran my test. You can see the response times are much better, even with 5ms of latency:
http://dishevelled.net/marmotta-fine-locking-all-responses.png
I've got a patch for this that I'm happy to send along. I drop the lock prior to running the getTripleId() query (allowing multiple connections to run the query at once), then only take it again if I need to write a new triple ID to the registry:
