-
Notifications
You must be signed in to change notification settings - Fork 141
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
improve rpc request result performance #1586
base: kjs/sync-store
Are you sure you want to change the base?
Conversation
async () => { | ||
const requestHashes = requests.map((request) => | ||
crypto.createHash("md5").update(request).digest("hex"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@typedarray Need to be very careful that this matches the previous hashes that we generated by postgres. I added a test for this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems good, but we should double check
- Make sure any pre-processing we're doing (casing, sorting keys?) is the same as before
- Casing, we're storing this in a TEXT column
- Any possibility that the strings were changed by the driver before making it to PG (in the old imp)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The request
is preprocessed before making it to this function.
ponder/packages/core/src/sync/transport.ts
Lines 84 to 86 in 689156e
})).map((request) => | |
toLowerCase(JSON.stringify(orderObject(request))), | |
); |
// Note: we don't cache request that failed or returned "0x". See more about "0x" below. | ||
.filter( | ||
({ result }) => result?.success && result.returnData !== "0x", | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@typedarray I don't think we should cache failed requests and just re-request them
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Brilliant
…ponder into kjs/rpc-request-results
Description
Improved several performance characteristics surrounding cached rpc requests
request
column fromrpc_request_results
tablepruneRpcRequestResults
multicall
requests