Improve binary serialization performance - #1829
Merged
Merged
Conversation
decodeVariant and decodeMatrix both built their flat element array with Array.newInstance plus a per-element Array.set, resolving the array type through OpcUaDataType.getBackingClass. That is the hot path for every array-valued Variant on the wire, and it paid a reflective store with a runtime assignability check, a switch dispatch that could not be hoisted out of the loop, and an Object return that defeated inlining. Replace both with decodeBuiltinTypeArray, which switches on the builtin type id once, allocates the concrete array type, and fills it with plain array stores. The component types match OpcUaDataType.getBackingClass exactly, which callers depend on: Variant.getDataType and Matrix both derive the DataType from the array's component type, including when the array is empty. Element arrays stay boxed. Decoding to primitive arrays would be a larger win, but it would break callers that cast a Variant value to Integer[]. Incidentally fixes a crash: an array-encoded Variant carrying a builtin type id outside 1..25 reached Array.newInstance(null, length) and threw a raw NullPointerException. The switch's default branch now reports Bad_DecodingError. No behavior change for valid input: same values, same array component types. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
decodeStructArray and decodeStructMatrix filled their arrays with Array.set, which does a runtime assignability check on every element. The check is redundant: DataTypeCodec.decode is already declared to return UaStructuredType. Array.newInstance stays, because the component type is only known at runtime from codec.getType(), but the cast both methods already performed on the way out moves up to the allocation so the loop can use a plain array store. One failure mode shifts: a codec whose getType() is not a UaStructuredType subtype now fails with ClassCastException at allocation instead of IllegalArgumentException on the first element. Both are unchecked and signal the same misregistration. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
encodeVariant and encodeMatrix walked their elements with Array.get, which is reflective and, for a primitive array, allocates a box per element. Variants do hold primitive arrays: user code creates them, and OpcUaJsonDecoder produces them for every array-valued Variant it decodes. Add encodeBuiltinTypeArray, which switches on the builtin type id once and then runs a typed loop, writing primitive elements straight to the buffer instead of boxing them. Arrays of UaStructuredType, UaEnumeratedType, and OptionSetUInteger still need per-element conversion, so they keep the existing reflective loop; isPlainBuiltinArray gates the two paths on an exact component-type match, which also sends supertype- and subtype-typed arrays down the safe path. encodeEnumMatrix and encodeStructMatrix drop Array.get too. Their elements are already UaEnumeratedType and UaStructuredType, so array covariance makes a single cast of the whole array sufficient. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
decodeInt32 returns Integer because the UaDecoder contract requires it, so every internal caller that wants an int boxed the value and unboxed it again. That covers every array and string length prefix, every element of decodeDimensions and decodeMatrixDimensions, and the four DiagnosticInfo index fields. Read those directly from the buffer instead. Public signatures are untouched; the boxed decodeInt32 stays for the UaDecoder API, for decodeBuiltinType, and for filling Integer[], where the box is the point. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
StatusCode is a record over a long, so new StatusCode(decodeUInt32()) allocated a UInteger only to unwrap it. UInteger caches 0..255, so this allocated for every status code outside that range, which is every code that is not Good. Status codes appear in every DataValue and every per-operation service result. Read the value straight from the buffer instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The depth counter was an AtomicInteger, so every Variant and every DiagnosticInfo decode paid a compare-and-swap and its memory barriers on the way in and again on the way out. The atomicity bought nothing. A decoder instance is confined to one thread at a time, exactly like the mutable buffer field it sits next to: SerializationQueue shares an instance behind a TaskQueue that serializes decode tasks, and the client and server UASC handlers each own a channel-confined instance. Executor handoff supplies the happens-before edge between tasks. Use a plain int and document why it is safe, since an AtomicInteger beside a plain buffer field reads like deliberate thread-safety. Increment and decrement stay in try/finally so the counter still unwinds when a decode throws. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
UShort.valueOf allocated on every call, while UInteger has precached 0..255 and UByte has precached its whole range all along. Every NodeId holds a UShort namespace index, so decoding a NodeId, ExpandedNodeId, or QualifiedName allocated one, and namespace indexes are nearly always below 256. Mirror UInteger's scheme rather than inventing a new one: a VALUES array sized by a precacheSize system property, a getCached lookup on the valueOf overloads, and a readResolve so deserialization cannot introduce a duplicate of a cached value. The cache is declared before MIN and MAX so their initializers see it. Sharing instances is unobservable here: UShort is final, wraps a single final int, and compares by value. valueOf(String) and valueOf(short) now route through valueOf(int), which made the private String and short constructors dead; they are removed. Parsing, masking, and range-check behavior are unchanged. UByte needed no change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Store decoded structs without reflection introduced a (UaStructuredType[]) cast on the array allocated from DataTypeCodec.getType(), but getType() is unbounded and generic codecs report component types that are not UaStructuredType — the encoder still supports their Object[]-backed matrices. Those matrices now failed to decode with a raw ClassCastException. Keep the flat array as Object[]: element stores still enforce the codec's component type, and Matrix accepts any backing array. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Enum and struct matrix elements are always reference arrays, so a single (Object[]) cast covers both the well-known typed backing and the Object[] built by generic codecs. The reflective Array.get fallback duplicated each loop for no benefit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Cache low UShort values added a range check to readResolve, but rangeCheck throws NumberFormatException, which is neither declared nor an ObjectStreamException: a corrupt stream escaped the documented ObjectInputStream.readObject failure modes. Throw InvalidObjectException so deserialization errors surface as deserialization errors. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The system property parsing rules were copy-pasted and had already drifted (FIXME comments, differing caps). Parse in one package-private UNumber helper parameterized by property name, default, and cap. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Removes reflection and boxing from the hot paths of the binary encoder and decoder, and caches low unsigned values:
Array.get/Array.setFollow-up fixes and cleanups from review of the branch:
DataTypeCodec.getType()is not necessarily aUaStructuredTypesubtype)InvalidObjectExceptionfromUShort.readResolveinstead of an undeclaredNumberFormatExceptionTesting
spotless:checkand the fullstack-coretest suite pass.🤖 Generated with Claude Code