Skip to content
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

Allow configuring LRU cache size in app.toml #14

Closed
4 tasks
ethanfrey opened this issue Jan 10, 2020 · 3 comments · Fixed by #26
Closed
4 tasks

Allow configuring LRU cache size in app.toml #14

ethanfrey opened this issue Jan 10, 2020 · 3 comments · Fixed by #26
Assignees
Labels
API Fixes to the external client api

Comments

@ethanfrey
Copy link
Member

Summary

Node operators should be able to configure LRU cache size based on their memory availability

Problem Definition

Storing instances in the LRU will have no effect on the results (still deterministic), but should lower execution time at the cost of increased memory usage. We cannot pick universal parameters for this, so we should allow node operators to set it.

Proposal

There seems to be an app.toml that the node operator can configure some subjective values like halt-height and min-gas-fee or such. Let's add one more value:

[wasm]
lru_size = 10

The default should be 0, which means disabled. This value must be parsed at start and is passed into the wasm.NewKeeper function. We currently have hardcoded the value to 3: https://github.com/cosmwasm/wasmd/blob/master/x/wasm/internal/keeper/keeper.go#L44


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@ethanfrey ethanfrey added the API Fixes to the external client api label Jan 10, 2020
@workshub
Copy link

workshub bot commented Jan 13, 2020

This issue is now published on WorksHub. If you would like to work on this issue you can
start work on the WorksHub Issue Details page.

@ethanfrey
Copy link
Member Author

We should also allow configuring gas limit for "smart queries" in the app.toml.

This question was brought up in #22

@ethanfrey
Copy link
Member Author

ethanfrey commented Jan 14, 2020

Note, this is where the custom querier handlers get called in the sdk: https://github.com/cosmos/cosmos-sdk/blob/9a183ffbcc0163c8deb71c7fd5f8089a83e58f05/baseapp/abci.go#L441-L460

This is set somewhere in the ante-handler on deliver: https://github.com/cosmos/cosmos-sdk/blob/master/baseapp/baseapp.go#L519-L522

Context.KVStore will set the gas meter based on the last call to Context.SetGasMeter

Ideally, you will do something like this before running the query:

ctx := ctx.WithGasMeter(sdk.NewGasMeter(cfgDefinedQueryGasLimit))
store := ctx.KVStore(k.storeKey)

Note that we must set the gas limit on the ctx before creating the store.
Also, we should set the wasm gas limit like this:

       gas := gasForContract(ctx)
	res, err := k.wasmer.Execute(codeInfo.CodeHash, params, msgs, prefixStore, gas)
	if err != nil {
		return sdk.Result{}, types.ErrExecuteFailed(err)
	}
	consumeGas(ctx, res.GasUsed)

loloicci pushed a commit to loloicci/wasmd that referenced this issue Apr 19, 2023
CosmWasm#14)

* fix: fix the cmd error that does not recognize wasmvm library version.

* chore: add unittest

* chore: remove unittest

* dump: dump up wasmvm v1.1.1-0.11.2

* doc: update changelog.

* chore: add more unittest
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Fixes to the external client api
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants