Skip to content

Make message clear() O(1) with presence-guarded getters #11

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
merlimat merged 2 commits into from
Jul 29, 2026
Merged

Make message clear() O(1) with presence-guarded getters #11

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
667e3e3
Make message clear() O(1) with presence-guarded getters
merlimat Jul 29, 2026
e4a02aa
Treat set-to-default as unset in proto3 copyFrom
merlimat Jul 29, 2026
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
31 changes: 23 additions & 8 deletions ...-generator/src/main/java/io/streamnative/lightproto/generator/LightProtoBooleanField.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,17 +27,32 @@ public LightProtoBooleanField(ProtoFieldDescriptor field, int index) {
public void getter(PrintWriter w) {
w.format(" /** Returns the value of the {@code %s} field. */\n", field.getName());
w.format(" public %s %s() {\n", field.getJavaType(), Util.camelCase("is", ccName));
w.format(" if (!(%s)) {\n", presenceCondition());
w.format(" return %s;\n", defaultValueExpr());
w.format(" }\n");
w.format(" return %s;\n", ccName);
w.format(" }\n");
}

@Override
public void clear(PrintWriter w) {
protected String defaultValueExpr() {
if (field.isDefaultValueSet()) {
w.format("%s = %s;\n", ccName, field.getDefaultValueAsString());
} else {
w.format("%s = false;\n", ccName);
return field.getDefaultValueAsString();
}
return "false";
}

@Override
protected String cmpValueExpr(String qualifier) {
if (field.hasImplicitPresence()) {
return qualifier + Util.camelCase("is", ccName) + "()";
}
return qualifier + ccName;
}

@Override
public void clear(PrintWriter w) {
// No value reset needed: the getter is guarded by the presence condition.
}

@Override
Expand Down Expand Up @@ -65,16 +80,16 @@ public void parseTextFormat(PrintWriter w) {

@Override
public void equalsCode(PrintWriter w) {
w.format("if (%s != _other.%s) return false;\n", ccName, ccName);
w.format("if (%s != %s) return false;\n", cmpValueExpr(""), cmpValueExpr("_other."));
}

@Override
public void hashCodeCode(PrintWriter w) {
w.format("_h = 31 * _h + (%s ? 1231 : 1237);\n", ccName);
w.format("_h = 31 * _h + (%s ? 1231 : 1237);\n", cmpValueExpr(""));
}

@Override
protected String nonDefaultCondition() {
return ccName;
protected String nonDefaultCondition(String qualifier) {
return qualifier + ccName;
}
}
22 changes: 13 additions & 9 deletions code-generator/src/main/java/io/streamnative/lightproto/generator/LightProtoBytesField.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ public void declaration(PrintWriter w) {

@Override
public void parse(PrintWriter w) {
// Invalidate any stale buffer reference from a previous parse or set: clear()
// no longer resets it, so it must be dropped when the field is on the wire.
w.format("%s = null;\n", ccName);
w.format("_%sLen = LightProtoCodec.readVarInt(_buffer);\n", ccName);
w.format("_%sIdx = _buffer.readerIndex();\n", ccName);
w.format("_buffer.skipBytes(_%sLen);\n", ccName);
Expand Down Expand Up @@ -65,13 +68,13 @@ public void setter(PrintWriter w, String enclosingType) {
public void getter(PrintWriter w) {
w.format("/** Returns the size in bytes of the {@code %s} field. */\n", field.getName());
w.format("public int %s() {\n", Util.camelCase("get", ccName, "size"));
w.format(" if (_%sLen < 0) { return 0; }\n", ccName);
w.format(" if (!(%s)) { return 0; }\n", presenceCondition());
w.format(" return _%sLen;\n", ccName);
w.format("}\n");

w.format("/** Returns the {@code %s} field as a byte array. */\n", field.getName());
w.format("public byte[] %s() {\n", Util.camelCase("get", ccName));
w.format(" if (_%sLen < 0) { return new byte[0]; }\n", ccName);
w.format(" if (!(%s)) { return new byte[0]; }\n", presenceCondition());
w.format(" io.netty.buffer.ByteBuf _b = %s();\n", Util.camelCase("get", ccName, "slice"));
w.format(" byte[] res = new byte[_b.readableBytes()];\n");
w.format(" _b.getBytes(0, res);\n");
Expand All @@ -80,7 +83,7 @@ public void getter(PrintWriter w) {

w.format("/** Returns the {@code %s} field as a ByteBuf slice. */\n", field.getName());
w.format("public io.netty.buffer.ByteBuf %s() {\n", Util.camelCase("get", ccName, "slice"));
w.format(" if (_%sLen < 0) { return io.netty.buffer.Unpooled.EMPTY_BUFFER; }\n", ccName);
w.format(" if (!(%s)) { return io.netty.buffer.Unpooled.EMPTY_BUFFER; }\n", presenceCondition());
w.format(" if (%s == null) {\n", ccName);
w.format(" return _parsedBuffer.slice(_%sIdx, _%sLen);\n", ccName, ccName);
w.format(" } else {\n");
Expand All @@ -90,15 +93,14 @@ public void getter(PrintWriter w) {
}

@Override
protected String nonDefaultCondition() {
return "_" + ccName + "Len > 0";
protected String nonDefaultCondition(String qualifier) {
return qualifier + "_" + ccName + "Len > 0";
}

@Override
public void clear(PrintWriter w) {
w.format("%s = null;\n", ccName);
w.format("_%sIdx = -1;\n", ccName);
w.format("_%sLen = -1;\n", ccName);
// No value reset needed: the getters are guarded by the presence condition, and
// parse() invalidates the stale buffer reference for fields present on the wire.
}

@Override
Expand Down Expand Up @@ -159,7 +161,9 @@ public void serialize(PrintWriter w) {

@Override
public void materialize(PrintWriter w) {
w.format("if (_%sIdx >= 0) {\n", ccName);
// The presence guard is required: an absent field may hold a stale buffer
// index from an earlier parse of a buffer that has since been released.
w.format("if ((%s) && _%sIdx >= 0) {\n", presenceCondition(), ccName);
w.format(" byte[] _tmp = new byte[_%sLen];\n", ccName);
w.format(" _parsedBuffer.getBytes(_%sIdx, _tmp);\n", ccName);
w.format(" %s = io.netty.buffer.Unpooled.wrappedBuffer(_tmp);\n", ccName);
Expand Down
23 changes: 15 additions & 8 deletions code-generator/src/main/java/io/streamnative/lightproto/generator/LightProtoEnumField.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,17 +36,24 @@ public void declaration(PrintWriter w) {
public void getter(PrintWriter w) {
w.format(" /** Returns the value of the {@code %s} field. */\n", field.getName());
w.format(" public %s %s() {\n", field.getJavaType(), Util.camelCase("get", field.getName()));
w.format(" if (!(%s)) {\n", presenceCondition());
w.format(" return %s;\n", defaultValueExpr());
w.format(" }\n");
w.format(" return %s;\n", ccName);
w.format(" }\n");
}

@Override
public void clear(PrintWriter w) {
protected String defaultValueExpr() {
if (field.isDefaultValueSet()) {
w.format("%s = %s;\n", ccName, field.getDefaultValueAsString());
} else {
w.format("%s = %s.valueOf(0);\n", ccName, field.getJavaType());
return field.getDefaultValueAsString();
}
return String.format("%s.valueOf(0)", field.getJavaType());
}

@Override
public void clear(PrintWriter w) {
// No value reset needed: the getter is guarded by the presence condition.
}

@Override
Expand Down Expand Up @@ -86,16 +93,16 @@ public void parseTextFormat(PrintWriter w) {

@Override
public void equalsCode(PrintWriter w) {
w.format("if (%s != _other.%s) return false;\n", ccName, ccName);
w.format("if (%s != %s) return false;\n", cmpValueExpr(""), cmpValueExpr("_other."));
}

@Override
public void hashCodeCode(PrintWriter w) {
w.format("_h = 31 * _h + %s.getValue();\n", ccName);
w.format("_h = 31 * _h + %s.getValue();\n", cmpValueExpr(""));
}

@Override
protected String nonDefaultCondition() {
return ccName + ".getValue() != 0";
protected String nonDefaultCondition(String qualifier) {
return qualifier + ccName + ".getValue() != 0";
}
}
40 changes: 29 additions & 11 deletions code-generator/src/main/java/io/streamnative/lightproto/generator/LightProtoField.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,11 @@ public void tags(PrintWriter w) {
w.format(" private static final int %s = %d;\n", fieldNumber(), field.getNumber());
w.format(" private static final int %s = (%s << LightProtoCodec.TAG_TYPE_BITS) | %s;\n", tagName(), fieldNumber(), typeTag());
w.format(" private static final int %s_SIZE = LightProtoCodec.computeVarIntSize(%s);\n", tagName(), tagName());
if (!field.isRepeated() && !field.isOneofMember() && !field.hasImplicitPresence()) {
// Presence bits are tracked for all non-repeated, non-oneof fields. For proto3
// implicit-presence fields the bit is internal (no has() method): it marks that the
// field was written since the last clear(), so clear() does not need to physically
// reset field values.
if (!field.isRepeated() && !field.isOneofMember()) {
w.format(" private static final int %s = 1 << (%d %% 32);\n", fieldMask(), index);
}
}
Expand All @@ -115,16 +119,16 @@ public void has(PrintWriter w) {
public void fieldClear(PrintWriter w, String enclosingType) {
w.format(" /** Clear the {@code %s} field. */\n", field.getName());
w.format(" public %s %s() {\n", enclosingType, Util.camelCase("clear", field.getName()));
// The value clear must run while presence is still set: message-field clear()
// is guarded by has(), so clearing the presence first would leave a stale child.
if (field.isOneofMember()) {
w.format(" if (_%sCase == %s) {\n", Util.camelCase(field.getOneofName()), fieldNumber());
w.format(" _%sCase = 0;\n", Util.camelCase(field.getOneofName()));
clear(w);
w.format(" _%sCase = 0;\n", Util.camelCase(field.getOneofName()));
w.format(" }\n");
} else if (!field.hasImplicitPresence()) {
w.format(" _bitField%d &= ~%s;\n", bitFieldIndex(), fieldMask());
clear(w);
} else {
clear(w);
w.format(" _bitField%d &= ~%s;\n", bitFieldIndex(), fieldMask());
}
w.format(" return this;\n");
w.format(" }\n");
Expand Down Expand Up @@ -219,13 +223,24 @@ protected int bitFieldIndex() {
return index / 32;
}

protected void writeSetPresence(PrintWriter w) {
if (field.hasImplicitPresence()) {
return; // No presence tracking for proto3 implicit presence
/**
* Returns the Java expression that is true when this (non-repeated) field is present:
* the oneof case check for oneof members, otherwise the presence bit check. Getters use
* this to return defaults for absent fields, since clear() no longer resets field values.
*/
protected String presenceCondition() {
if (field.isOneofMember()) {
return String.format("_%sCase == %s", Util.camelCase(field.getOneofName()), fieldNumber());
}
return String.format("(_bitField%d & %s) != 0", bitFieldIndex(), fieldMask());
}

protected void writeSetPresence(PrintWriter w) {
if (field.isOneofMember()) {
w.format(" _%sCase = %s;\n", Util.camelCase(field.getOneofName()), fieldNumber());
} else {
// Also set for proto3 implicit presence: the internal bit guards against
// stale values surviving an O(1) clear().
w.format(" _bitField%d |= %s;\n", bitFieldIndex(), fieldMask());
}
}
Expand All @@ -239,16 +254,19 @@ public String serializeCondition() {
return null;
}
if (field.hasImplicitPresence()) {
return nonDefaultCondition();
// The presence bit guards against stale values after an O(1) clear();
// the non-default check preserves proto3 semantics (defaults not emitted).
return presenceCondition() + " && " + nonDefaultCondition("");
}
return Util.camelCase("has", field.getName()) + "()";
}

/**
* Returns the Java expression that is true when this field has a non-default value.
* Returns the Java expression that is true when this field's value, qualified by the
* given prefix (e.g. "_other."), is not the proto3 default.
* Only called for proto3 implicit presence fields.
*/
protected String nonDefaultCondition() {
protected String nonDefaultCondition(String qualifier) {
throw new UnsupportedOperationException("nonDefaultCondition not implemented for " + getClass().getSimpleName());
}

Expand Down
44 changes: 14 additions & 30 deletions code-generator/src/main/java/io/streamnative/lightproto/generator/LightProtoMapField.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,15 @@ public void parse(PrintWriter w) {

// Ensure capacity before parsing (message values parse directly into the array)
w.format("_ensure%sCapacity();\n", Util.camelCaseFirstUpper(ccName));
if (isMessageValue()) {
// Reset the pooled slot up front: if the entry's value field is absent from
// the wire, the slot must yield defaults, not data from a previous cycle.
w.format("if (_%sValues[_%sCount] == null) {\n", ccName, ccName);
w.format(" _%sValues[_%sCount] = new %s();\n", ccName, ccName, valueField.getJavaType());
w.format("} else {\n");
w.format(" _%sValues[_%sCount].clear();\n", ccName, ccName);
w.format("}\n");
}

// Parse entry fields
w.format("while (_buffer.readerIndex() < _%sEntryEnd) {\n", ccName);
Expand Down Expand Up @@ -450,10 +459,8 @@ private void generateValueTempParse(PrintWriter w) {
w.format(" _%sValueIdx = _buffer.readerIndex();\n", ccName);
w.format(" _buffer.skipBytes(_%sValueLen);\n", ccName);
} else if (isMessageValue()) {
// The slot was already allocated and reset right after _ensureCapacity().
w.format(" int _%sMsgSize = LightProtoCodec.readVarInt(_buffer);\n", ccName);
w.format(" if (_%sValues[_%sCount] == null) {\n", ccName, ccName);
w.format(" _%sValues[_%sCount] = new %s();\n", ccName, ccName, valueField.getJavaType());
w.format(" }\n");
w.format(" _%sValues[_%sCount].parseFrom(_buffer, _%sMsgSize);\n", ccName, ccName, ccName);
} else {
w.format(" _%sValue = %s;\n", ccName, LightProtoNumberField.parseNumber(valueField));
Expand Down Expand Up @@ -970,33 +977,10 @@ public void copy(PrintWriter w) {

@Override
public void clear(PrintWriter w) {
if (isStringKey()) {
w.format("for (int _i = 0; _i < _%sCount; _i++) {\n", ccName);
w.format(" LightProtoCodec.StringHolder _sh = _%sKeys[_i];\n", ccName);
w.format(" _sh.s = null;\n");
w.format(" _sh.idx = -1;\n");
w.format(" _sh.len = -1;\n");
w.format("}\n");
}
if (isStringValue()) {
w.format("for (int _i = 0; _i < _%sCount; _i++) {\n", ccName);
w.format(" LightProtoCodec.StringHolder _sh = _%sValues[_i];\n", ccName);
w.format(" _sh.s = null;\n");
w.format(" _sh.idx = -1;\n");
w.format(" _sh.len = -1;\n");
w.format("}\n");
} else if (isBytesValue()) {
w.format("for (int _i = 0; _i < _%sCount; _i++) {\n", ccName);
w.format(" LightProtoCodec.BytesHolder _bh = _%sValues[_i];\n", ccName);
w.format(" _bh.b = null;\n");
w.format(" _bh.idx = -1;\n");
w.format(" _bh.len = -1;\n");
w.format("}\n");
} else if (isMessageValue()) {
w.format("for (int _i = 0; _i < _%sCount; _i++) {\n", ccName);
w.format(" _%sValues[_i].clear();\n", ccName);
w.format("}\n");
}
// Entries beyond the count are unreachable. Both parse() and put() write the
// complete key/value holder state (including nulling stale s/b references),
// and message values are cleared by parseFrom()/put() when slots are reused,
// so no per-entry reset is needed here.
w.format("_%sCount = 0;\n", ccName);
w.format("_%sIndex = null;\n", ccName);
}
Expand Down
Loading
Loading