Skip to content

Optimized Map/Set lookups in Structure #1246

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

Merged
merged 1 commit into from
Aug 17, 2020
Merged

Optimized Map/Set lookups in Structure #1246

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 @@ -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
---------
Expand Down
66 changes: 45 additions & 21 deletions src/com/sun/jna/Structure.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down Expand Up @@ -157,7 +168,7 @@ public interface ByReference { }
private Map<String, StructField> structFields;
// Keep track of native C strings which have been allocated,
// corresponding to String fields of this Structure
private final Map<String, Object> nativeStrings = new HashMap<String, Object>();
private final Map<String, NativeStringTracking> nativeStrings = new HashMap<String, NativeStringTracking>(8);
private TypeMapper typeMapper;
// This field is accessed by native code
private long typeInfo;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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());
}

Expand Down Expand Up @@ -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++) {
Expand Down Expand Up @@ -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);
}
Expand All @@ -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());
}
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()) {
Expand Down Expand Up @@ -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 {
Expand Down