Remove Log slices to pointers, use Sonic #178
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR makes minor improvements by replacing the use of the standard JSON package with sonic’s default configuration and converting log slices from pointers ( []*types.Log ) to value types ( []types.Log ) where appropriate.
- Replace standard json marshaling/unmarshaling with sonic’s ConfigDefault variants across multiple files.
- Update signatures and implementations to use value types for logs, reducing unnecessary pointer usage.
Reviewed Changes
Copilot reviewed 70 out of 70 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| ethrpc/ethrpc.go | Updated JSON operations to use sonic and added sonic import. |
| ethrpc/batch.go | Replaced json marshalling with sonic marshalling. |
| ethreceipts/receipt.go | Changed logs field and related methods from pointer slices to value slices. |
| ethreceipts/filterer.go | Updated filter functions to use value slices for logs. |
| ethreceipts/ethreceipts.go | Updated log group and count logic; replaced pointer log handling with value-based handling. |
| ethmonitor/* & others | Replaced json usage with sonic across various components. |
| etcoder/, ethartifact/, cmd/* | Adopted sonic for JSON operations and updated log handling where applicable. |
Comments suppressed due to low confidence (1)
ethreceipts/ethreceipts.go:948
- The function 'max' is referenced here but is not defined or imported. Consider replacing it with a defined max operation, for example using a custom utility function or an inline comparison.
blockCount = max(blockCount, log.TxIndex+1)
pkieltyka
left a comment
pkieltyka
left a comment
There was a problem hiding this comment.
can you switch to sonic.ConfigFastest instead of sonic.ConfigDefault
| // Find specific meta transaction -- note: this is not the "transaction hash", | ||
| // this is a sub-transaction where the id is emitted as an event. | ||
| FilterMetaTransactionID := func(metaTxnID ethkit.Hash) ethreceipts.FilterQuery { | ||
| return ethreceipts.FilterLogs(func(logs []*types.Log) bool { |
There was a problem hiding this comment.
can you write a little benchmark separately to see what the performance effects are of doing this..? I'm curious if this will save us memory in practice..?
There was a problem hiding this comment.
After writing a benchmark neither pointer removal or sonic help with performance.
klaidliadon
changed the title
|
After some benchmarking this changes don't bring any improvement in performance. Closing it in favor of #179 which contains the salvageable parts of this PR. |
Uh oh!
There was an error while loading. Please reload this page.