diff --git a/CHANGES.md b/CHANGES.md index 328db4a134..251cc5e963 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -12,6 +12,7 @@ Features * [#1237](https://github.com/java-native-access/jna/pull/1237): *Experimental:* Add artifacts that make jna and jna-platform named modules (provide `module-info.class`). The new artifacts are named `jna-jpms.jar` and `jna-platform-jpms.jar` - [@matthiasblaesing](https://github.com/matthiasblaesing). * [#1242](https://github.com/java-native-access/jna/pull/1242): Add CallWindowProc to User32 - [@heldplayer](https://github.com/heldplayer). * [#1239](https://github.com/java-native-access/jna/pull/1239): Improve performance of allocation of `c.s.j.Memory` objects - [@joerg1985](https://github.com/joerg1985). +* [#1246](https://github.com/java-native-access/jna/pull/1246): Improve performance of `c.s.j.Structure#read` and `c.s.j.Structure#write` - [@joerg1985](https://github.com/joerg1985). Bug Fixes --------- diff --git a/src/com/sun/jna/Structure.java b/src/com/sun/jna/Structure.java index 13433644b8..92cf8eaf92 100644 --- a/src/com/sun/jna/Structure.java +++ b/src/com/sun/jna/Structure.java @@ -127,6 +127,17 @@ public interface ByValue { } */ public interface ByReference { } + /** A class to keep NativeString instances alive and avoid writing the same value again and again */ + private static class NativeStringTracking { + + private final Object value; + private NativeString peer; + + NativeStringTracking(Object lastValue) { + this.value = lastValue; + } + } + /** Use the platform default alignment. */ public static final int ALIGN_DEFAULT = 0; /** No alignment, place all fields on nearest 1-byte boundary */ @@ -157,7 +168,7 @@ public interface ByReference { } private Map structFields; // Keep track of native C strings which have been allocated, // corresponding to String fields of this Structure - private final Map nativeStrings = new HashMap(); + private final Map nativeStrings = new HashMap(8); private TypeMapper typeMapper; // This field is accessed by native code private long typeInfo; @@ -343,14 +354,15 @@ void useMemory(Pointer m, int offset, boolean force) { this.memory.write(0, buf, 0, buf.length); } else { - // Ensure our memory pointer is initialized, even if we can't - // yet figure out a proper size/layout - this.memory = m.share(offset); if (size == CALCULATE_SIZE) { size = calculateSize(false); } if (size != CALCULATE_SIZE) { this.memory = m.share(offset, size); + } else { + // Ensure our memory pointer is initialized, even if we can't + // yet figure out a proper size/layout + this.memory = m.share(offset); } } this.array = null; @@ -438,6 +450,8 @@ public int size() { /** Clears the native memory associated with this Structure. */ public void clear() { ensureAllocated(); + // ensure the memory is released and the values are written again + nativeStrings.clear(); memory.clear(size()); } @@ -508,8 +522,9 @@ public boolean add(Structure o) { if (!contains(o)) { ensureCapacity(count+1); elements[count++] = o; + return true; } - return true; + return false; } private int indexOf(Structure s1) { for (int i=0;i < count;i++) { @@ -579,10 +594,9 @@ public void read() { ensureAllocated(); // Avoid redundant reads - if (busy().contains(this)) { + if (!busy().add(this)) { return; } - busy().add(this); if (this instanceof Structure.ByReference) { reading().put(getPointer(), this); } @@ -593,7 +607,7 @@ public void read() { } finally { busy().remove(this); - if (reading().get(getPointer()) == this) { + if (this instanceof Structure.ByReference && reading().get(getPointer()) == this) { reading().remove(getPointer()); } } @@ -740,8 +754,18 @@ protected Object readField(StructField structField) { if (fieldType.equals(String.class) || fieldType.equals(WString.class)) { - nativeStrings.put(structField.name + ".ptr", memory.getPointer(offset)); - nativeStrings.put(structField.name + ".val", result); + if (result != null) { + NativeStringTracking current = new NativeStringTracking(result); + NativeStringTracking previous = nativeStrings.put(structField.name, current); + + if (previous != null) { + // regardless of value changed or not, keep the old native string alive + current.peer = previous.peer; + } + } else { + // the value is cleared, we don't need to keep the native string alive + nativeStrings.remove(structField.name); + } } // Update the value on the Java field @@ -769,10 +793,9 @@ public void write() { } // Avoid redundant writes - if (busy().contains(this)) { + if (!busy().add(this)) { return; } - busy().add(this); try { // Write all fields, except those marked 'volatile' for (StructField sf : fields().values()) { @@ -840,28 +863,29 @@ protected void writeField(StructField structField) { // Java strings get converted to C strings, where a Pointer is used if (String.class == fieldType || WString.class == fieldType) { - // Allocate a new string in memory - boolean wide = fieldType == WString.class; if (value != null) { + NativeStringTracking current = new NativeStringTracking(value); + NativeStringTracking previous = nativeStrings.put(structField.name, current); + // If we've already allocated a native string here, and the // string value is unchanged, leave it alone - if (nativeStrings.containsKey(structField.name + ".ptr") - && value.equals(nativeStrings.get(structField.name + ".val"))) { + if (previous != null && value.equals(previous.value)) { + // value is unchanged, keep the old native string alive + current.peer = previous.peer; return; } + // Allocate a new string in memory + boolean wide = fieldType == WString.class; NativeString nativeString = wide ? new NativeString(value.toString(), true) : new NativeString(value.toString(), encoding); - // Keep track of allocated C strings to avoid - // premature garbage collection of the memory. - nativeStrings.put(structField.name, nativeString); + // value is changed, keep the new native string alive + current.peer = nativeString; value = nativeString.getPointer(); } else { nativeStrings.remove(structField.name); } - nativeStrings.remove(structField.name + ".ptr"); - nativeStrings.remove(structField.name + ".val"); } try {