Skip to content

Commit

Permalink
Fixes #1625 issue by caching the field list and validation state in a…
Browse files Browse the repository at this point in the history 
…ddition to the existing layoutInfo and fieldOrder caches.
  • Loading branch information
@brettwooldridge
brettwooldridge committed Sep 22, 2024
1 parent 50b1cbf commit db67c42
Show file tree
Hide file tree
Showing 2 changed files with 102 additions and 26 deletions.
1 change: 1 addition & 0 deletions CHANGES.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ Next Release (5.16.0)

Features
--------
* [#1626](https://github.com/java-native-access/jna/pull/1626): Add caching of field list and field validation in `Structure` along with more efficient reentrant read-write locking instead of synchronized() blocks - [@BrettWooldridge](https://github.com/brettwooldridge)

Bug Fixes
---------
Expand Down
127 changes: 101 additions & 26 deletions src/com/sun/jna/Structure.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
import java.util.Map;
import java.util.Set;
import java.util.WeakHashMap;
import java.util.concurrent.locks.ReentrantReadWriteLock;
import java.util.logging.Level;
import java.util.logging.Logger;

Expand Down Expand Up @@ -155,8 +156,14 @@ private static class NativeStringTracking {
//public static final int ALIGN_8 = 6;

protected static final int CALCULATE_SIZE = -1;
static final ReentrantReadWriteLock layoutInfoLock = new ReentrantReadWriteLock();
static final ReentrantReadWriteLock fieldOrderLock = new ReentrantReadWriteLock();
static final ReentrantReadWriteLock fieldListLock = new ReentrantReadWriteLock();
static final ReentrantReadWriteLock validationLock = new ReentrantReadWriteLock();
static final Map<Class<?>, LayoutInfo> layoutInfo = new WeakHashMap<>();
static final Map<Class<?>, List<String>> fieldOrder = new WeakHashMap<>();
static final Map<Class<?>, List<Field>> fieldList = new WeakHashMap<>();
static final Map<Class<?>, Boolean> validationMap = new WeakHashMap<>();

// This field is accessed by native code
private Pointer memory;
Expand Down Expand Up @@ -1015,36 +1022,68 @@ protected void sortFields(List<Field> fields, List<String> names) {
* this {@link Structure} class.
*/
protected List<Field> getFieldList() {
List<Field> flist = new ArrayList<>();
for (Class<?> cls = getClass();
!cls.equals(Structure.class);
cls = cls.getSuperclass()) {
List<Field> classFields = new ArrayList<>();
Field[] fields = cls.getDeclaredFields();
for (int i=0;i < fields.length;i++) {
int modifiers = fields[i].getModifiers();
if (Modifier.isStatic(modifiers) || !Modifier.isPublic(modifiers)) {
continue;
}
classFields.add(fields[i]);
Class<?> clazz = getClass();
// Try to read the value under the read lock
fieldListLock.readLock().lock();
try {
List<Field> fields = fieldList.get(clazz);
if (fields != null) {
return fields; // Return the cached result if found
}
flist.addAll(0, classFields);
} finally {
fieldListLock.readLock().unlock();
}

// If not found, compute the value under the write lock
fieldListLock.writeLock().lock();
try {
// Double-check if another thread has computed the value before we do
return fieldList.computeIfAbsent(clazz, (c) -> {
List<Field> flist = new ArrayList<>();
List<Field> classFields = new ArrayList<>();
for (Class<?> cls = clazz;
!cls.equals(Structure.class);
cls = cls.getSuperclass()) {
for (Field field : cls.getDeclaredFields()) {
int modifiers = field.getModifiers();
if (Modifier.isStatic(modifiers) || !Modifier.isPublic(modifiers)) {
continue;
}
classFields.add(field);
}
flist.addAll(0, classFields);
classFields.clear();
}
return flist;
});
} finally {
fieldListLock.writeLock().unlock();
}
return flist;
}

/** Cache field order per-class.
* @return (cached) ordered list of fields
*/
private List<String> fieldOrder() {
Class<?> clazz = getClass();
synchronized(fieldOrder) {
List<String> list = fieldOrder.get(clazz);
if (list == null) {
list = getFieldOrder();
fieldOrder.put(clazz, list);
// Try to read the value under the read lock
fieldOrderLock.readLock().lock();
try {
List<String> order = fieldOrder.get(clazz);
if (order != null) {
return order; // Return the cached result if found
}
return list;
} finally {
fieldOrderLock.readLock().unlock();
}

// If not found, compute the value under the write lock
fieldOrderLock.writeLock().lock();
try {
// Double-check if another thread has computed the value before we do (see JavaDoc)
return fieldOrder.computeIfAbsent(clazz, (c) -> getFieldOrder());
} finally {
fieldOrderLock.writeLock().unlock();
}
}

Expand Down Expand Up @@ -1159,8 +1198,11 @@ static int size(Class<? extends Structure> type) {
*/
static <T extends Structure> int size(Class<T> type, T value) {
LayoutInfo info;
synchronized(layoutInfo) {
layoutInfoLock.readLock().lock();
try {
info = layoutInfo.get(type);
} finally {
layoutInfoLock.readLock().unlock();
}
int sz = (info != null && !info.variable) ? info.size : CALCULATE_SIZE;
if (sz == CALCULATE_SIZE) {
Expand All @@ -1183,8 +1225,11 @@ int calculateSize(boolean force, boolean avoidFFIType) {
int size = CALCULATE_SIZE;
Class<?> clazz = getClass();
LayoutInfo info;
synchronized(layoutInfo) {
layoutInfoLock.readLock().lock();
try {
info = layoutInfo.get(clazz);
} finally {
layoutInfoLock.readLock().unlock();
}
if (info == null
|| this.alignType != info.alignType
Expand All @@ -1196,7 +1241,8 @@ int calculateSize(boolean force, boolean avoidFFIType) {
this.structFields = info.fields;

if (!info.variable) {
synchronized(layoutInfo) {
layoutInfoLock.readLock().lock();
try {
// If we've already cached it, only override layout if
// we're using non-default values for alignment and/or
// type mapper; this way we don't override the cache
Expand All @@ -1205,8 +1251,18 @@ int calculateSize(boolean force, boolean avoidFFIType) {
if (!layoutInfo.containsKey(clazz)
|| this.alignType != ALIGN_DEFAULT
|| this.typeMapper != null) {
// Must release read lock before acquiring write lock (see JavaDoc lock escalation example)
layoutInfoLock.readLock().unlock();
layoutInfoLock.writeLock().lock();

layoutInfo.put(clazz, info);

// Downgrade by acquiring read lock before releasing write lock (again, see JavaDoc)
layoutInfoLock.readLock().lock();
layoutInfoLock.writeLock().unlock();;
}
} finally {
layoutInfoLock.readLock().unlock();
}
}
size = info.size;
Expand Down Expand Up @@ -1250,9 +1306,28 @@ private void validateField(String name, Class<?> type) {

/** ensure all fields are of valid type. */
private void validateFields() {
List<Field> fields = getFieldList();
for (Field f : fields) {
validateField(f.getName(), f.getType());
// Try to read the value under the read lock
validationLock.readLock().lock();
try {
if (validationMap.containsKey(getClass())) {
return; // Return because this Structure has already been validated
}
} finally {
validationLock.readLock().unlock();
}

// If not found, perform validation and update the cache under the write lock
validationLock.writeLock().lock();
try {
// Double-check if another thread has computed the value before we do (see JavaDoc)
validationMap.computeIfAbsent(getClass(), (cls) -> {
for (Field f : getFieldList()) {
validateField(f.getName(), f.getType());
}
return true;
});
} finally {
validationLock.writeLock().unlock();
}
}

Expand Down

0 comments on commit db67c42

Please sign in to comment.