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

worker: some code enhancement on work.go #1174

Merged

Conversation

setunapo
Copy link
Contributor

@setunapo setunapo commented Nov 10, 2022

Description

The code of work.go is critical for BSC block generation.
Try to do some code enhancement for it:
1.the resubmit intervalAdjust code was for PoW consensus, can be deleted.
2.change the work interrupt from atomic check to channel communication.

Rationale

With PoW, there will be a periodic timer to check if it is the time to stop packing transaction and start calculating the desired hash value, since other miner could succeed in hash compute if it spends too much time packing transactions.
It will commit the current fruit to calculate root at a reasonable time.
And it will schedule a new work to get a big block if new transaction was received.

When there are too many transactions in the TxPool, the interval of the resubmit timer would be increased and vice versa.

But it is not needed with PoS consensus, since the block interval is determined in PoS and there is already a timer to stop packing for too long.

Example

None

Changes

Remove unused worker code for BSC.

qinglin89
qinglin89 previously approved these changes Nov 10, 2022
@setunapo setunapo force-pushed the worker_no_resubmit_interval_adjust branch from 956182d to bbb132f Compare November 10, 2022 03:55
@setunapo setunapo changed the title WIP: remove resubmit ajust code of PoW. worker: remove resubmit adjust code of PoW. Nov 10, 2022
@setunapo setunapo force-pushed the worker_no_resubmit_interval_adjust branch 2 times, most recently from bcc2e22 to 188c5ca Compare November 10, 2022 07:15
@brilliant-lx brilliant-lx added the r4r ready for review label Nov 10, 2022
@brilliant-lx brilliant-lx changed the base branch from master to develop November 10, 2022 07:18
@brilliant-lx brilliant-lx dismissed qinglin89’s stale review November 10, 2022 07:18

The base branch was changed.

resubmit intervalAdjust is for PoW only, to remove it to make worker simpler.

With PoW, there will be a periodic timer to check if it is the time to stop
packing transaction and start calculating the desired hash value, since other miner
could succeed in hash compute if it spends too much time packing transactions.
It will commit the current fruit to calculate root at a reasonable time.
And it will schedule a new work to get a big block if new transaction was received.

When there are too many transactions in the TxPool, the interval of the resubmit timer would be
increased and vice versa.

But it is not needed with PoS related consensus, since the block interval is determined in PoS,
and there is already a timer to stop too long packing.
@setunapo setunapo force-pushed the worker_no_resubmit_interval_adjust branch from 188c5ca to 5eff61c Compare November 10, 2022 07:20
brilliant-lx
brilliant-lx previously approved these changes Nov 10, 2022
@brilliant-lx brilliant-lx changed the title worker: remove resubmit adjust code of PoW. worker: some code enhancement on work.go Nov 10, 2022
bitdaddy89
bitdaddy89 previously approved these changes Nov 10, 2022
channel operation is preferred than atomic value check in golang.
And it will help for the further refactor on worker.
Copy link
Contributor

@KeefeL KeefeL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@brilliant-lx brilliant-lx merged commit d1ed977 into bnb-chain:develop Nov 15, 2022
@setunapo setunapo deleted the worker_no_resubmit_interval_adjust branch November 16, 2022 03:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
r4r ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants