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

[Feature] Implement read VM pool #546

Merged
merged 16 commits into from
Oct 6, 2021
Merged

[Feature] Implement read VM pool #546

merged 16 commits into from
Oct 6, 2021

Conversation

yun-yeo
Copy link
Contributor

@yun-yeo yun-yeo commented Aug 30, 2021

Summary of changes

close #545

It also contains necessary dependency updates for multi-reader thread implementation.

WASM Config changes

New Configs

  • write-vm-memory-cache-size
  • read-vm-memory-cache-size
  • num-read-vms

Removed Configs

  • contract-memory-cache-size
[wasm]
# The maximum gas amount can be spent for contract query.
# The contract query will invoke contract execution vm,
# so we need to restrict the max usage to prevent DoS attack
contract-query-gas-limit = "3000000"

# The flag to specify whether print contract logs or not
contract-debug-mode = "false"

# The write WASM VM memory cache size in MiB not bytes
write-vm-memory-cache-size = "500"

# The read WASM VM memory cache size in MiB not bytes
read-vm-memory-cache-size = "30"

# The number of read WASM VMs
num-read-vms = "5"

We need this two PRs before this.

Report of required housekeeping

  • Github issue OR spec proposal link
  • Wrote tests
  • Updated API documentation (client/lcd/swagger-ui/swagger.yaml)
  • Added a relevant changelog entry: clog add [section] [stanza] [message]

(FOR ADMIN) Before merging

  • Added appropriate labels to PR
  • Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)
  • Confirm added tests are consistent with the intended behavior of changes
  • Ensure all tests pass

@yun-yeo yun-yeo added enhancement New feature or request good first issue Good for newcomers wasm Wasm contract related update labels Aug 30, 2021
@yun-yeo yun-yeo requested a review from hanjukim August 30, 2021 08:31
@yun-yeo yun-yeo self-assigned this Aug 30, 2021
@yun-yeo yun-yeo changed the title test purpose readvm pool implementation, still requiring tendermint R… [Feature] Implement read VM pool Aug 30, 2021
@codecov
Copy link

codecov bot commented Aug 31, 2021

Codecov Report

Merging #546 (60311bb) into main (f3a1b82) will increase coverage by 0.11%.
The diff coverage is 77.41%.

❗ Current head 60311bb differs from pull request most recent head c27e974. Consider uploading reports for the commit c27e974 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##             main     #546      +/-   ##
==========================================
+ Coverage   44.66%   44.77%   +0.11%     
==========================================
  Files         120      121       +1     
  Lines        6914     6957      +43     
==========================================
+ Hits         3088     3115      +27     
- Misses       3588     3596       +8     
- Partials      238      246       +8     
Impacted Files Coverage Δ
x/wasm/keeper/legacy_querier.go 34.48% <60.00%> (+0.74%) ⬆️
x/wasm/keeper/querier.go 35.38% <60.00%> (+0.95%) ⬆️
x/wasm/keeper/keeper.go 81.30% <77.77%> (-4.90%) ⬇️
x/wasm/keeper/pool.go 83.33% <83.33%> (ø)
x/wasm/keeper/contract.go 78.59% <100.00%> (+0.25%) ⬆️
x/oracle/tally.go 86.48% <0.00%> (-5.41%) ⬇️

@kjessec
Copy link
Contributor

kjessec commented Sep 6, 2021

should we merge this to #551?

@hanjukim hanjukim removed the good first issue Good for newcomers label Sep 15, 2021
@yun-yeo
Copy link
Contributor Author

yun-yeo commented Sep 20, 2021

This should be merged after cosmos-sdk be multi-tread safe (IAVL update)

hanjukim
hanjukim previously approved these changes Sep 22, 2021
@yun-yeo yun-yeo requested a review from hanjukim October 6, 2021 01:29
@yun-yeo yun-yeo mentioned this pull request Oct 6, 2021
8 tasks
@yun-yeo yun-yeo merged commit 6e903ef into main Oct 6, 2021
@yun-yeo yun-yeo deleted the feature/readvm-pool branch October 6, 2021 02:55
octalmage pushed a commit to octalmage/core that referenced this pull request Oct 21, 2021
* test purpose readvm pool implementation, still requiring tendermint RWMutex

* fix lint

* remove data race

* remove replace

* wrap pool logic with mutex

* fix comment

* enforce zero readvm config to use default config

* bump cosmos-sdk to v0.44.1

* bump cosmwasm to v0.16.1

* cleanup go.mod

* changelog update

* prevent zero write vm cache

* rename the functions

* fix typo
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request wasm Wasm contract related update
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE] Create read VM pool to increase query performance
3 participants