-
Notifications
You must be signed in to change notification settings - Fork 140
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.
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
Change Encoder to use new CBOR library Streaming API (Phase 1) #796
Comments
@turbolent here's the proof-of-concept code I used. Most notable changes are in And these two functions were combined:
git diff encode.go
|
@fxamacker The diff look good! Great idea to document the structure that is encoded, as it is now not as obvious anymore. Let me know when a branch is ready on the CBOR library side (no rush) and I'll make these changes here on the Cadence |
@turbolent thanks! Updating/debugging fuzz tests for streaming encoder today took longer than expected. Round-trip fuzz tests on randomly generated data can get interesting if the same data can be encoded in different ways and still be valid. Fuzzing passed 500,000 execs now (~15 mins on a throttled cpu)...I'll have the branch ready either tonight or 1st thing in the morning. |
Very cool! It's not urgent, please take your time 🙂 |
Used streaming mode for encoding to eliminate intermediate objects being created when encoding large composites and dictionaries. Used low-level API for encoding to reduce CBOR library's internal calls to Go's reflect package (to boost speed). Modify go.mod to use fxamacker/cbor branch feature/stream-mode before it's merged to master. Closes onflow#794 Closes onflow#796 Closes onflow#807
Used streaming mode for encoding to eliminate intermediate objects being created when encoding large composites and dictionaries. Used low-level API for encoding to reduce CBOR library's internal calls to Go's reflect package (to boost speed). Modify go.mod to use fxamacker/cbor branch feature/stream-mode before it's merged to master. Closes #794 Closes #796 Closes #807
Issue To Be Solved
The Cadence storage layer can be optimized to encode faster by using a new streaming encoding API of and fxamacker/cbor, being introduced in #795.
Suggestion
By using streaming mode for encoding, we can eliminate intermediate objects being created when encoding large composites and dictionaries. Other worthwhile optimizations are also possible by providing a low-level API in fxamacker/cbor.
Preliminary benchmark comparisons show performance gains are possible by encoding to CBOR in streaming mode. Adding streaming mode to fxamacker/cbor was one of the ideas proposed by @dete and @turbolent during our first chat.
The effort required to add streaming mode has been simplified by replacing all
CBORMap
withCBORArray
in the various PRs for issue #738. We don't need to deal with maps. 🎉Proposed Changes
Modify the Cadence storage layer to use the new streaming encoding API, specifically Phase 1.
💡 Phase 1 provides a new API that helps eliminate the creation of intermediate objects when encoding large arrays, composites, and dictionaries. Tests using unoptimized new API show encoding is 12% faster for a 776,155 byte 'Value'. Memory usage improved more than speed.
Caveats
The streaming encoding API will not perform validation, so keep the validation step that is already performed currently after encoding.
The text was updated successfully, but these errors were encountered: