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

chain: set the gc_fork_clean_step default to 100 #6647

Merged
merged 3 commits into from
Apr 26, 2022

Conversation

marcelo-gonzalez
Copy link
Contributor

The current default has shown itself to be quite expensive at times,
sometimes taking ~half a second to go through all 1000. Drop the default to
100 to try and mitigate this.

Note that since most people won't have set this new config option manually,
this will be a real change for most.

The current default has shown itself to be quite expensive at times,
sometimes taking ~half a second to go through all 1000. Drop the default to
100 to try and mitigate this.

Note that since most people won't have set this new config option manually,
this will be a real change for most.
@nagisa
Copy link
Collaborator

nagisa commented Apr 20, 2022

I've little exposure to this code to review quickly. I can try and make some time to familiarize with the relevant portions, but if landing this soon is a concern, you'd be better served by finding somebody else to review this ^^

@marcelo-gonzalez
Copy link
Contributor Author

I've little exposure to this code to review quickly. I can try and make some time to familiarize with the relevant portions, but if landing this soon is a concern, you'd be better served by finding somebody else to review this ^^

@nagisa No problem, I've actually been discussing this with @mina86 and @mm-near, so they already have context on this and have even tested the value some

@marcelo-gonzalez marcelo-gonzalez removed the request for review from nagisa April 20, 2022 16:14
@bowenwang1996
Copy link
Collaborator

@marcelo-gonzalez I thought we wanted this change in the 1.26.0 release. Did I miss something?

@marcelo-gonzalez
Copy link
Contributor Author

@marcelo-gonzalez I thought we wanted this change in the 1.26.0 release. Did I miss something?

yeah, didn't realize at the time that it broke several tests so will include it in an -rc2 after I fix those

@near-bulldozer near-bulldozer bot merged commit ab04de4 into near:master Apr 26, 2022
marcelo-gonzalez added a commit that referenced this pull request Apr 27, 2022
The current default has shown itself to be quite expensive at times,
sometimes taking ~half a second to go through all 1000. Drop the default to
100 to try and mitigate this.

Note that since most people won't have set this new config option manually,
this will be a real change for most.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants