Skip to content

Commit cbdaee6

Browse files
benalexaubenalexau
committed
Update formatting, documentation and limit GC test write frequency
The GarbageCollectionTest performing 1,000 GC operations caused a substantial increase in test execution time. Trial and error found commenting out the new ReferenceUtil.reachabilityFence0 references resulted in the failures being consistently detected with a value of 250. Nevertheless even a value of 250 would result in the new test being approximately 3 times longer in duration than any other test. To resolve this the default value was reduced to 50 and a system property introduced to allow any arbitrary value to be used. In turn the GitHub Action was amended to test with 1,000 GC-inducing writes as per the original code. As an aside on OpenJDK Java 20 the mere presence of a call to ReferenceUtil.reachabilityFence0 was sufficient to pass the tests (even with high values such as 2,000) despite there being no method body remaining in reachabilityFence0. As such it is unverified whether the synchronization is actually required for the strong reachability. This is consistent with apache/datasketches-memory#91 (comment). In any event synchronization has been retained given it is the more conservative approach.
1 parent c5e02f7 commit cbdaee6

File tree

3 files changed

+116
-98
lines changed

3 files changed

+116
-98
lines changed

‎.github/workflows/maven.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ jobs:
2929
run: ./cross-compile.sh
3030

3131
- name: Build with Maven
32-
run: mvn -B verify
32+
run: mvn -B verify -DgcRecordWrites=1000
3333

3434
- name: Store built native libraries for later jobs
3535
uses: actions/upload-artifact@v3

‎src/main/java/org/lmdbjava/ReferenceUtil.java

Lines changed: 40 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -22,34 +22,46 @@
2222

2323
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
2424

25+
/**
26+
* Supports creating strong references in manner compatible with Java 8.
27+
*/
2528
public final class ReferenceUtil {
26-
/**
27-
* Ensures that the object referenced by the given reference remains
28-
* <a href="package-summary.html#reachability"><em>strongly reachable</em></a>,
29-
* regardless of any prior actions of the program that might otherwise cause
30-
* the object to become unreachable; thus, the referenced object is not
31-
* reclaimable by garbage collection at least until after the invocation of
32-
* this method.
33-
*
34-
* <p> Recent versions of the JDK have a nasty habit of prematurely deciding objects are unreachable.
35-
* see: https://stackoverflow.com/questions/26642153/finalize-called-on-strongly-reachable-object-in-java-8
36-
* The Java 9 method Reference.reachabilityFence offers a solution to this problem.
37-
*
38-
* <p> This method is always implemented as a synchronization on {@code ref}, not as
39-
* {@code Reference.reachabilityFence} for consistency across platforms and to allow building on JDK 6-8.
40-
* <b>It is the caller's responsibility to ensure that this synchronization will not cause deadlock.</b>
41-
*
42-
* @param ref the reference. If {@code null}, this method has no effect.
43-
* @see https://github.com/netty/netty/pull/8410
44-
*/
45-
@SuppressFBWarnings({"ESync_EMPTY_SYNC", "UC_USELESS_VOID_METHOD"})
46-
public static void reachabilityFence0(Object ref) {
47-
if (ref != null) {
48-
synchronized (ref) {
49-
// Empty synchronized is ok: https://stackoverflow.com/a/31933260/1151521
50-
}
51-
}
52-
}
5329

54-
private ReferenceUtil() {}
30+
private ReferenceUtil() {
31+
}
32+
33+
/**
34+
* Ensures that the object referenced by the given reference remains
35+
* <em>strongly reachable</em>, regardless of any prior actions of the program
36+
* that might otherwise cause the object to become unreachable. Thus, the
37+
* referenced object is not reclaimable by garbage collection at least until
38+
* after the invocation of this method.
39+
*
40+
* <p>
41+
* Recent versions of the JDK have a nasty habit of prematurely deciding
42+
* objects are unreachable (eg
43+
* <a href="https://tinyurl.com/so26642153">StackOverflow question
44+
* 26642153</a>.
45+
*
46+
* <p>
47+
* <code>java.lang.ref.Reference.reachabilityFence</code> offers a solution to
48+
* this problem, but it was only introduced in Java 9. LmdbJava presently
49+
* supports Java 8 and therefore this method provides an alternative.
50+
*
51+
* <p>
52+
* This method is always implemented as a synchronization on {@code ref}.
53+
* <b>It is the caller's responsibility to ensure that this synchronization
54+
* will not cause deadlock.</b>
55+
*
56+
* @param ref the reference (null is acceptable but has no effect)
57+
* @see <a href="https://github.com/netty/netty/pull/8410">Netty PR 8410</a>
58+
*/
59+
@SuppressFBWarnings({"ESync_EMPTY_SYNC", "UC_USELESS_VOID_METHOD"})
60+
public static void reachabilityFence0(final Object ref) {
61+
if (ref != null) {
62+
synchronized (ref) {
63+
// Empty synchronized is ok: https://stackoverflow.com/a/31933260/1151521
64+
}
65+
}
66+
}
5567
}

‎src/test/java/org/lmdbjava/GarbageCollectionTest.java

Lines changed: 75 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -20,92 +20,98 @@
2020

2121
package org.lmdbjava;
2222

23+
import static java.nio.ByteBuffer.allocateDirect;
24+
import static java.nio.charset.StandardCharsets.UTF_8;
25+
import static org.junit.Assert.fail;
26+
import static org.lmdbjava.DbiFlags.MDB_CREATE;
27+
import static org.lmdbjava.Env.create;
28+
29+
import java.io.File;
30+
import java.io.IOException;
31+
import java.nio.ByteBuffer;
32+
2333
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
2434
import org.junit.Rule;
2535
import org.junit.Test;
2636
import org.junit.rules.TemporaryFolder;
2737
import org.mockito.MockedStatic;
2838
import org.mockito.Mockito;
2939

30-
import java.io.File;
31-
import java.io.IOException;
32-
import java.nio.ByteBuffer;
33-
34-
import static java.nio.ByteBuffer.allocateDirect;
35-
import static java.nio.charset.StandardCharsets.UTF_8;
36-
import static org.junit.Assert.fail;
37-
import static org.lmdbjava.DbiFlags.MDB_CREATE;
38-
import static org.lmdbjava.Env.create;
39-
4040
@SuppressFBWarnings({"DM_GC", "RV_RETURN_VALUE_IGNORED_NO_SIDE_EFFECT"})
4141
@SuppressWarnings("PMD.DoNotCallGarbageCollectionExplicitly")
4242
public class GarbageCollectionTest {
4343

44-
private static final String DB_NAME = "my DB";
45-
private static final String KEY_PREFIX = "Uncorruptedkey";
46-
private static final String VAL_PREFIX = "Uncorruptedval";
44+
private static final String DB_NAME = "my DB";
45+
private static final String KEY_PREFIX = "Uncorruptedkey";
46+
private static final String VAL_PREFIX = "Uncorruptedval";
4747

48-
@Rule
49-
public final TemporaryFolder tmp = new TemporaryFolder();
50-
private void putBuffer(Dbi<ByteBuffer> db, Txn<ByteBuffer> txn, int i) {
51-
ByteBuffer key = allocateDirect(24);
52-
ByteBuffer val = allocateDirect(24);
53-
key.put((KEY_PREFIX+i).getBytes(UTF_8)).flip();
54-
val.put((VAL_PREFIX+i).getBytes(UTF_8)).flip();
55-
db.put(txn, key, val);
56-
}
57-
@Test
58-
public void buffersNotGarbageCollectedTest() throws IOException {
59-
final File path = tmp.newFolder();
48+
@Rule
49+
public final TemporaryFolder tmp = new TemporaryFolder();
6050

61-
final Env<ByteBuffer> env = create()
62-
.setMapSize(2_085_760_999)
63-
.setMaxDbs(1)
64-
.open(path);
65-
final Dbi<ByteBuffer> db = env.openDbi(DB_NAME, MDB_CREATE);
51+
@Test
52+
public void buffersNotGarbageCollectedTest() throws IOException {
53+
final File path = tmp.newFolder();
54+
try (Env<ByteBuffer> env = create()
55+
.setMapSize(2_085_760_999)
56+
.setMaxDbs(1)
57+
.open(path)) {
58+
final Dbi<ByteBuffer> db = env.openDbi(DB_NAME, MDB_CREATE);
6659

67-
// Trigger compilation and whatnot
68-
try (Txn<ByteBuffer> txn = env.txnWrite()) {
69-
for (int i = 0; i < 5000; i++) {
70-
putBuffer(db, txn, i);
71-
}
72-
txn.commit();
60+
try (Txn<ByteBuffer> txn = env.txnWrite()) {
61+
for (int i = 0; i < 5_000; i++) {
62+
putBuffer(db, txn, i);
7363
}
74-
// Call gc before writing to lmdb and after last reference to buffer by changing the behavior of mask
75-
try (MockedStatic<MaskedFlag> mockedStatic = Mockito.mockStatic(MaskedFlag.class)) {
76-
mockedStatic.when(MaskedFlag::mask).thenAnswer(invocationOnMock -> {
77-
System.gc();
78-
return 0;
79-
});
80-
try (Txn<ByteBuffer> txn = env.txnWrite()) {
81-
for (int i = 0; i < 1000; i++) {
82-
putBuffer(db, txn, i);
83-
}
84-
txn.commit();
85-
}
64+
txn.commit();
65+
}
66+
67+
// Call GC before writing to LMDB and after last reference to buffer by
68+
// changing the behavior of mask
69+
try (MockedStatic<MaskedFlag> mockedStatic = Mockito.mockStatic(
70+
MaskedFlag.class)) {
71+
mockedStatic.when(MaskedFlag::mask).thenAnswer(invocationOnMock -> {
72+
System.gc();
73+
return 0;
74+
});
75+
final int gcRecordWrites = Integer.getInteger("gcRecordWrites", 50);
76+
try (Txn<ByteBuffer> txn = env.txnWrite()) {
77+
for (int i = 0; i < gcRecordWrites; i++) {
78+
putBuffer(db, txn, i);
79+
}
80+
txn.commit();
8681
}
82+
}
8783

88-
// Find corrupt keys
89-
try (Txn<ByteBuffer> txn = env.txnRead()) {
90-
try (Cursor<ByteBuffer> c = db.openCursor(txn)) {
91-
if (c.first()) {
92-
do {
93-
byte[] rkey = new byte[c.key().remaining()];
94-
c.key().get(rkey);
95-
byte[] rval = new byte[c.val().remaining()];
96-
c.val().get(rval);
97-
String skey = new String(rkey, UTF_8);
98-
String sval = new String(rval, UTF_8);
99-
if (!skey.startsWith("Uncorruptedkey")) {
100-
fail("Found corrupt key " + skey);
101-
}
102-
if (!sval.startsWith("Uncorruptedval")) {
103-
fail("Found corrupt val " + sval);
104-
}
105-
} while (c.next());
106-
}
107-
}
84+
// Find corrupt keys
85+
try (Txn<ByteBuffer> txn = env.txnRead()) {
86+
try (Cursor<ByteBuffer> c = db.openCursor(txn)) {
87+
if (c.first()) {
88+
do {
89+
final byte[] rkey = new byte[c.key().remaining()];
90+
c.key().get(rkey);
91+
final byte[] rval = new byte[c.val().remaining()];
92+
c.val().get(rval);
93+
final String skey = new String(rkey, UTF_8);
94+
final String sval = new String(rval, UTF_8);
95+
if (!skey.startsWith("Uncorruptedkey")) {
96+
fail("Found corrupt key " + skey);
97+
}
98+
if (!sval.startsWith("Uncorruptedval")) {
99+
fail("Found corrupt val " + sval);
100+
}
101+
} while (c.next());
102+
}
108103
}
109-
env.close();
104+
}
110105
}
106+
}
107+
108+
private void putBuffer(final Dbi<ByteBuffer> db, final Txn<ByteBuffer> txn,
109+
final int i) {
110+
final ByteBuffer key = allocateDirect(24);
111+
final ByteBuffer val = allocateDirect(24);
112+
key.put((KEY_PREFIX + i).getBytes(UTF_8)).flip();
113+
val.put((VAL_PREFIX + i).getBytes(UTF_8)).flip();
114+
db.put(txn, key, val);
115+
}
116+
111117
}

0 commit comments

Comments
 (0)