Improve omgidl deserialization performance #123
Conversation
There was a problem hiding this comment.
Great work on this! That definitely more of an improvement than I would've expected for this work.
Would be great to pull out the deserializer builders/cache to a separate class and write a few tests for them. Then for the class we can explain why it's necessary in the context of this performance improvement.
packages/omgidl-serialization/src/DeserializationInfoCache.test.ts
Outdated
Show resolved
Hide resolved
| reader: CdrReader, | ||
| options: HeaderOptions, | ||
| /** The size of the struct if known (like from an emHeader). If it is known we do not read in a dHeader */ | ||
| knownTypeSize?: number, | ||
| flags: { knownTypeSize?: number }, |
There was a problem hiding this comment.
we should either join this into options or keep as a number to avoid instantiating new object instances that need to be garbage collected.
There was a problem hiding this comment.
Just noticed that the value of knownTypeSize is never used, it is only checked if it is defined. Is/was this intended?
There was a problem hiding this comment.
This was intended. However could be good information to have if we want to skip deserializing parts of messages in the future.
|
|
||
| // fieldLength only used for `string` type primitives | ||
| return deser(reader, headerSpecifiedLength); | ||
| private readComplexNestedArray( |
There was a problem hiding this comment.
is readNestedArray necessary anymore?
There was a problem hiding this comment.
Yes, this is used for reading typed arrays
**User-Facing Changes** Improve omgidl deserialization performance **Description** Bumps `@foxglove/omgidl-serialization` which improves deserialization performance (foxglove/omgidl#123)
Public-Facing Changes
Improve omgidl deserialization performance
Description
This PR changes the
MessageReaderto build a tree structure of deserialization info objects when constructed. When building this tree, the concrete deserializers as well as other relevant type information are looked up which avoids that this has to be done during the actual deserialization. Further changes include the removal of some temporary closures which reduces some GC time.The outcome of these changes is a performance improvement of up to 30% (depends on message type)
In the image above, left is this PR, right is the currently released version
Resolves FG-6300