Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Improve Structure (subclass) construction performance. #1626

Merged
merged 1 commit into from
Oct 1, 2024
Merged

Improve Structure (subclass) construction performance. #1626

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES.md
View file
Open in desktop
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
Open in desktop
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
Loading