-
Notifications
You must be signed in to change notification settings - Fork 683
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
feat(api): add a cost_tracker option to callreadonly rpc specifying the CostTracker to use for evaluation #5828
base: develop
Are you sure you want to change the base?
Conversation
…ling callreadonly, choosing from 'mid_block' (default; current) and 'free'
So there's a couple questions about the benchmark that I think need to be resolved before thinking about adding this option. First -- the benchmarks linked use a MemoryBackingStore. This means that the cost of loading the "cost contracts" is nearly free. In the actual stacks node's RPC interface, it has to load/initialize the cost contracts by reading from the stacks chain state. This could be much more costly, and could be the dominating factor for the performance difference seen between a Free tracker (which does not need to load/initialize the cost contracts) and the limited tracker. If that's the case, then the problem for RPC endpoints could be resolved by caching the cost contracts (this would be nice because it would speed up the RPC endpoint for people running public nodes as well). Second -- it's important to evaluate this sort of thing with a release build. The release build shows similar performance differences that increase with complexity, but the overhead is itself much smaller in real terms:
A difference of 7 ms is definitely a real difference, but was the total overhead of the RPC endpoint in the use case you were seeing just 7 ms? Or was it more significant? |
Thanks for taking a look at this @kantai
Agreed - loading the contracts could be a significant cost and isn't measured in the benchmark. I'll look into running a fork with either server-timing headers in the callreadonly response (measuring the contract loading time and the evaluation time separately) or with these timings in the logs. On caching as a potential solution: in callreadonly, we load the chain tip before we instantiate the CostTracker, so we could cache the To construct the LimitedCostTracker with the cached TrackerData fields, I think the simplest change would be to add a public method to LimitedCostTracker: TrackerData fields are private (from the perspective of the stackslib package) and I think the latter would be the natural place to define the cache. This new method could work by allowing the caller to pass the
Good point - I'll update the README to include the The benchmark is designed to show the relationship between contract complexity, cost tracker and execution time; as noted in the README, the absolute numbers are somewhat meaningless.
The original motivation for running the benchmark and exploring this option was that I had observed a speed up from ~30ms to ~7ms by switching from the non-free to the free LimitedCostTracker. It's hard to replicate this in a way that I can share - and I imagine the results will vary drastically based on the contract. Hence the benchmark. TL;DRI'll try running another experiment in a fork (off 3.1.0.0.3), logging evaluation times and contract loading times on mainnet with both the free and non-free cost trackers and run some traffic with a range of contracts. If you have any ideas for which contracts would be good to test - let me know! |
Great! If it turns out to be the case that caching would get us most of the performance benefit here, I'd suggest trying to make the caching system work. If switching to the free tracker ends up still being necessary, then my suggestion would be to refactor this PR somewhat. Ultimately, we want the node operator to be able to configure this (rather than the client), which means a new config setting will have to be plumbed through the |
For the contracts I'm working with, I'm seeing the following median times (μs) for executing the following parts of the
In this case:
Two outstanding questions I have:
|
@kantai: what do you reckon to me continuing with your suggested refactor given my findings on mainnet? I can also include an approach to caching and reusing the cost contracts to shave off some of that 1ms cost tracker initialisation. |
Yeah, I think that refactoring the configuration to support the free tracker makes sense in this case. |
@alexjtupper -- the latest Updating to use the latest revision of clarity/stacks-common in your benchmark: diff --git a/Cargo.toml b/Cargo.toml
index 120525f..a7504ff 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -4,7 +4,7 @@ version = "0.1.0"
edition = "2021"
[dependencies]
-clarity = { git = "https://github.com/stacks-network/stacks-core", tag = "3.1.0.0.5", version = "0.0.1", features = [
+clarity = { git = "https://github.com/stacks-network/stacks-core", rev = "3a935b5c8d92550f520812cca9655b407269cfa7", features = [
'testing',
] }
-stacks-common = { git = "https://github.com/stacks-network/stacks-core", tag = "3.1.0.0.5", version = "0.0.1" }
+stacks-common = { git = "https://github.com/stacks-network/stacks-core", rev = "3a935b5c8d92550f520812cca9655b407269cfa7" } The measured runtimes seem much closer now: the overhead is more like 10-12%.
Is this performance close enough that you no longer want to configure the free tracker? |
Description
When evaluating a Clarity contract function, Clarity tracks its runtime cost using a CostTracker. The choice of CostTracker can significantly impact the time it takes to complete the evaluation (execution time).
This PR adds an optional
cost_tracker
query parameter to the callreadonly RPC endpoint, allowing the client to specify which CostTracker to use, potentially allowing the client to choose a faster evaluation.Applicable issues
None
Additional info (benefits, drawbacks, caveats)
Currently, the callreadonly RPC endpoint uses a LimitedCostTracker initialised with
LimitedCostTracker::new_mid_block()
. At initialisation, this LimitedCostTracker loads the on-chain contracts specifying how costs are calculated from the store. Then, during contract evaluation, this CostTracker will evaluate a cost function for each cost-incurring operation during evaluation. By doing so, the callreadonly endpoint, through this LimitedCostTracker, can enforce cost-related limits during execution.An alternative to the above LimitedCostTracker, is
LimitedCostTracker::Free
. This cost tracker doesn't load the on-chain cost-related contracts, and returns 0 cost which computing the cost of each cost-incurring operation during evaluation. It doesn't evaluate any cost functions and it can't enforce any cost-related limits.This benchmark, comparing execution times of contract evaluation (through the Environment interface) with both
OwnedEnvironment::new_free()
andOwnedEnvironment::new_max_limit()
, shows that using LimitedCostTracker::Free can result in significantly faster execution. It shows a significant (10-20x) increase in execution time when using the default, non-free CostTracker, and that this increase is positively related to the number of operations in the function (crudely measured as the number of executed function calls in the benchmarked contract function). YMMV.Example benchmark output:
The speed-up from using LimitedCostTracker::Free may be particularly valuable to people running semi-private follower nodes for the purpose of having their own RPC endpoint (i.e. a node that is p2p publicly connected but for which the RPC endpoint is intended for internal use). The reduction in execution time from this change could result in significantly higher throughput for the callreadonly endpoint for a given target latency. However, this comes at the cost of not being able to enforce cost-related limits.
Considerations
This PR takes the approach of allowing the client to specify which of the two aforementioned LimitedCostTracker initialisations to use (via a
cost_tracker
query parameter) - giving the client the ability to make the tradeoff between performance and limit enforcement for their use-case.However, it is not clear whether this could be abused on nodes with public facing RPC endpoints - with public clients deciding to evaluate contracts without cost control.
A change we could consider is whether to enable / disable this query parameter option in the node's config. When disabled, the query parameter could be ignored - falling back to the current, default behaviour.
Checklist
docs/rpc/openapi.yaml
andrpc-endpoints.md
for v2 endpoints,event-dispatcher.md
for new events)clarity-benchmarking
repobitcoin-tests.yml