Skip to content

Commit

Permalink
Merge pull request #1626 from brettwooldridge/master
Browse files Browse the repository at this point in the history 
Improve ``Structure`` (subclass) construction performance.
  • Loading branch information
@matthiasblaesing
matthiasblaesing authored Oct 1, 2024
2 parents 50b1cbf + db67c42 commit c9e3895
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 c9e3895

Please sign in to comment.