chore: bring notecard COBS optimizations into note-c#216
Conversation
There was a problem hiding this comment.
Pull request overview
This PR brings significant performance optimizations to the COBS (Consistent Overhead Byte Stuffing) encoding and decoding functions in note-c. The optimizations replace byte-by-byte processing with bulk memory operations and hardware-accelerated searches, claiming 10x-70x performance improvements for large data transfers.
Changes:
- Replaced byte-by-byte COBS decoding with bulk memory operations (memmove/memcpy) for improved performance
- Replaced byte-by-byte COBS encoding with memchr()-based zero-finding and bulk copy operations
- Updated _cobsEncodedLength() to use the same optimization strategy as encoding
- Added comprehensive inline documentation explaining the optimization techniques
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
⏺ Analysis Complete After thoroughly studying both implementations and the COBS specification, I can report that the feedback is incorrect The Key Finding Both the Go and C implementations produce 256 bytes for encoding 254 non-zero input bytes. This is the correct COBS Detailed Trace of C Implementation (254 non-zero bytes) Looking at cobs.c:18-88: Initial: length=254, code=1, code_ptr=dst[0], dst advanced to position 1 Loop iteration:
Condition check:
Loop exits (length = 0) Post-loop: Write code (1) to code_ptr (position 255) Output: [0xFF, data×254, 0x01] = 256 bytes Go Implementation Verification The Go test file at hub/note-go/notecard/cobs_test.go:198-215 explicitly confirms this is expected: // 254 non-zero bytes: [0xFF, 254 data bytes, 0x01] Why 256 Bytes is Correct Per the COBS specification (from the original https://stuartcheshire.org/papers/COBSforToN.pdf): "To encode some bytes, first append a zero byte, then break them into groups..." For input [1, 2, ..., 254]:
cobsEncodedLength Verification Tracing cobs.c:94-137 for 254 non-zero bytes:
Both functions return 256. There is no mismatch. The Feedback's Error The feedback suggests changing else if (code == 0xFF) to else if (code == 0xFF && length > 0). This would break the
The trailing 0x01 is required to properly terminate the encoded stream. It signals "0 data bytes follow" which CobsEncodedLength Formula Confirmation Go uses: length + (1 + (length / 254))
Conclusion
The suggested "fix" in the feedback would actually break standard COBS compliance and could cause interoperability Sources: |
These are huge optimizations that impact large transfers - arguably 10x-70x.
8515d76 to
6bf0a99
Compare
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
These are huge optimizations that impact large transfers - arguably 10x-70x.