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.

Sign up for GitHub

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

Jump to bottom

sp cli: make sectors list much faster #10202

Merged
merged 3 commits into from
Feb 15, 2023
Merged

sp cli: make sectors list much faster #10202

merged 3 commits into from
Feb 15, 2023

Conversation

magik6k
Copy link
Contributor

@magik6k magik6k commented Feb 7, 2023

Related Issues

Proposed Changes

Make sector checks run in parallel

Additional Info

pre
./lotus-miner sectors list  31.82s user 1.64s system 59% cpu **56.525 total**
./lotus-miner sectors list --fast  34.44s user 1.97s system 329% cpu **11.060 total**


post
./lsm sectors list  15.59s user 1.72s system 248% cpu **6.971 total**
./lsm sectors list --fast  11.24s user 2.76s system 784% cpu **1.785 total**

Checklist

Before you mark the PR ready for review, please make sure that:

  • Commits have a clear commit message.
  • PR title is in the form of of <PR type>: <area>: <change being made>
    • example: fix: mempool: Introduce a cache for valid signatures
    • PR type: fix, feat, build, chore, ci, docs, perf, refactor, revert, style, test
    • area, e.g. api, chain, state, market, mempool, multisig, networking, paych, proving, sealing, wallet, deps
  • New features have usage guidelines and / or documentation updates in
  • Tests exist for new functionality or change in behavior
  • CI is green

@magik6k magik6k requested a review from a team as a code owner February 7, 2023 10:52
@magik6k magik6k force-pushed the feat/faster-sectors-list branch 3 times, most recently from 41bae08 to 9b58ac7 Compare February 7, 2023 18:17
@magik6k magik6k mentioned this pull request Feb 9, 2023
Closed
@magik6k magik6k marked this pull request as draft February 9, 2023 15:56
@magik6k magik6k force-pushed the feat/faster-sectors-list branch from 9b58ac7 to 17adcb7 Compare February 9, 2023 16:02
@magik6k magik6k marked this pull request as ready for review February 9, 2023 16:02
arajasek
arajasek approved these changes Feb 14, 2023
View reviewed changes
Copy link
Contributor

@arajasek arajasek left a comment

Choose a reason for hiding this comment

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

LGTM with one q

cmd/lotus-miner/sectors.go
},
Action: func(cctx *cli.Context) error {
minerApi, closer, err := lcli.GetStorageMinerAPI(cctx)
minerApi, closer, err := lcli.GetStorageMinerAPI(cctx, cliutil.StorageMinerUseHttp)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was getting bottlenecked by a single-threaded json encoder in websocket mode. In http mode all those requests are running on entirely separate goroutines, and don't get blocked on json encoding/decoding

Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting, thanks. Say that in a comment, please, then merge?

@magik6k magik6k enabled auto-merge February 15, 2023 18:42
@magik6k magik6k merged commit 285dfed into master Feb 15, 2023
@magik6k magik6k deleted the feat/faster-sectors-list branch February 15, 2023 18:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@arajasek arajasek arajasek approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@magik6k @arajasek