Repository navigation
perf(parser): parse segment fields on first read and keep received bytes - #42
Conversation
A segment now keeps its line on parse() and parses each field the first time it is read. Asking for the field table splits the line without building fields. A malformed field throws only when that field is read, and a failed read leaves no partial state. Refs SV-77
With the delimiters a segment was parsed with, an unread segment serializes as its received line. A read segment keeps each field's received bytes unless the field's content now encodes differently from what was received. Writing a field re-encodes only that field. Different delimiters still re-encode every field. Refs SV-77
Reading a field no longer rewrites a segment's leading parts from the serializing encoding, so a received MSH.2 that declares characters the encoding does not (such as a truncation character) survives a read. Round-trip tests now cover CRLF input and reading every field. The README says which Encoding the byte-exact guarantee needs. Refs SV-77
With fields parsed on first read, SV-83's repetition check fires on reading PID.1 rather than from parse(). Refs SV-77
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
The Robin Hood of code review
Heads up: this is a point-in-time review. Push fixes freely, then comment
/robinwhenever you want another pass.
Summary
The change adds lazy field materialization and preserves received wire bytes while providing focused tests for deferred validation and round-tripping. One reparse edge case remains: dynamically added field definitions survive parsing new, shorter content, so the segment can report fields that are no longer present.
Generic definitions added for a line's extra wire fields, or for reading past the schema, no longer outlive that line: parse() trims the field table back to the declared definitions. Refs SV-77
|
There was a problem hiding this comment.
The Robin Hood of code review
Heads up: this is a point-in-time review. Push fixes freely, then comment
/robinwhenever you want another pass.
✅ No issues found
Summary
The change cleanly defers field construction and validation while preserving received wire bytes, and it adds useful coverage for lazy reads, malformed values, and round-tripping. I found no actionable correctness or security issues in the reviewed changes.
Segment docblocks now say parse() defers field parsing, reads may throw for malformed fields, and serialize() keeps received bytes. The Unreleased entries share one benchmark baseline and form a single list. Refs SV-77
|
There was a problem hiding this comment.
The Robin Hood of code review
Heads up: this is a point-in-time review. Push fixes freely, then comment
/robinwhenever you want another pass.
✅ No issues found
Summary
The change adds lazy field parsing and preserves received wire bytes during serialization, with broad tests covering malformed fields, mutation, and round-tripping. The implementation and documentation are generally consistent, and I found no actionable correctness, security, or reliability issues in the reviewed diff.
1 participant
Goal
Parse a segment's fields only when they are read, so unread fields cost nothing, a malformed field throws only when that field is read, and
parse()followed byserialize()is byte-exact except for fields that actually changed, as SV-77 specifies.Changes
AbstractSegment::parse()keeps the line and itsEncoding. The line is split into wire field strings on first need, without building fields, and each field is parsed the first time it is read. A failed parse leaves no partial state, and the next read throws again.getFieldCount(),getNames(),isRequired()andmaxRepetitions()split the line without parsing fields, so generic segments and extra vendor fields report the same values as before.serialize()emits the received line for an unread segment when the delimiters match (Encoding::sameDelimiters(), which ignores the line ending). Otherwise each field keeps its received bytes unless its content now encodes differently. The received field count is kept (joinTrimmed()gains a$keepminimum). Different delimiters re-encode every field, as before.MSHoverrides only the field mapping (MSH-1 is the separator, MSH-2 is verbatim) and the header. Reading a field never rewrites a received MSH-2.Typelayer. This replaces the dirty-flag design from perf(parser): defer segment field parsing until first access #40, which is closed.maxRepsviolation when PID.1 is read, not fromparse().Encodingto serialize with, read-time errors) and CHANGELOG updated.On a 111-segment
ADT_A01with 100NK1(PHP 8.5, opcache): parse 5.3 → 0.7 ms; parse + serialize, unread, 28.2 → 0.7 ms; one PID-5 write changes 1 line instead of 105.Refs SV-77