Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.

Give state_col 90% of memory budget and fix other col calculation#4208

Merged
rphmeier merged 2 commits into
masterfrom
bkchr-db-fix-memory-budget
Nov 26, 2019
Merged

Give state_col 90% of memory budget and fix other col calculation#4208
rphmeier merged 2 commits into
masterfrom
bkchr-db-fix-memory-budget

Conversation

@bkchr
Copy link
Copy Markdown
Member

@bkchr bkchr commented Nov 26, 2019

No description provided.

@bkchr bkchr added the A0-please_review Pull request needs code review. label Nov 26, 2019
@bkchr bkchr requested a review from arkpar November 26, 2019 11:25
@arkpar
Copy link
Copy Markdown
Member

arkpar commented Nov 26, 2019

I suppose it would be good to increase default rocksdb cache size as well, as it appears to affect performance significantly.

@dvdplm
Copy link
Copy Markdown
Contributor

dvdplm commented Nov 26, 2019

@arkpar the default is the same 128Mb per column as before – do you suggest a higher value?

@bkchr
Copy link
Copy Markdown
Member Author

bkchr commented Nov 26, 2019

I could just increase the db_cache size in Substrate to 1024.

@arkpar
Copy link
Copy Markdown
Member

arkpar commented Nov 26, 2019

@dvdplm I was referring to the default value set by substrate/cli, but apparently it is None which uses DB_DEFAULT_COLUMN_MEMORY_BUDGET_MB. Strangely though we are observing significant performance regression in substrate, even though it is still using this default.

@bkchr Let's set it to 256Mib

@dvdplm
Copy link
Copy Markdown
Contributor

dvdplm commented Nov 26, 2019

Strangely though we are observing significant performance regression in substrate

On eth we're seeing some some terrible performance regressions on block import benchmarks too when using this pr: it's 10x slower. It could be related to the config changes and in that case it could impact substrate as well?

@arkpar
Copy link
Copy Markdown
Member

arkpar commented Nov 26, 2019

Strangely though we are observing significant performance regression in substrate

On eth we're seeing some some terrible performance regressions on block import benchmarks too when using this pr: it's 10x slower. It could be related to the config changes and in that case it could impact substrate as well?

Looks like it does, as we've merged paritytech/parity-common#257. Increasing cache size seems to help with performance though.

@rphmeier rphmeier merged commit 2b716b5 into master Nov 26, 2019
@rphmeier rphmeier deleted the bkchr-db-fix-memory-budget branch November 26, 2019 12:25
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

A0-please_review Pull request needs code review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants