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

test: investigate slow request handler imports #6481

Draft
wants to merge 2 commits into
base: v-next
Choose a base branch
from
Draft

Conversation

galargh
Copy link
Member

@galargh galargh commented Mar 18, 2025

  • Because this PR includes a bug fix, relevant tests have been included.
  • Because this PR includes a new feature, the change was previously discussed on an Issue or with someone from the team.
  • I didn't do anything of this.

Related to #6211

In 92ceb85, I set up a test which measures the import times of request handlers. It is not ideal but good enough to give us some data to work with.

By analyzing the results, it is clear that the Local Accounts Handler is the main culprit of the slow imports (the HD Wallet Accounts Handler imports the Local Accounts Handler which makes it slow by association).

As an exercise, I was able to shave off 1/3 of the loading time from the Local Accounts Handler by making the micro-eth-signer imports dynamic in 4d429a1. It shows that it is possible (and we could definitely do that for some other import paths there), but I'm not entirely sure we should go down that route.

I think we might be better off with the original solution to this, which is to use dynamic imports for handlers in the handlers array.

Here are the raw results of the handler import time analysis:

/**
* Example debug output:
* 2025-03-18T10:54:37.575Z hardhat:test:network-manager:request-handlers:request-array importing hd-wallet-handler.ts took 37ms
* 2025-03-18T10:54:37.576Z hardhat:test:network-manager:request-handlers:request-array importing local-accounts.ts took 36ms
* 2025-03-18T10:54:37.576Z hardhat:test:network-manager:request-handlers:request-array importing fixed-sender-handler.ts took 11ms
* 2025-03-18T10:54:37.576Z hardhat:test:network-manager:request-handlers:request-array importing chain-id-handler.ts took 11ms
* 2025-03-18T10:54:37.576Z hardhat:test:network-manager:request-handlers:request-array importing chain-id.ts took 11ms
* 2025-03-18T10:54:37.576Z hardhat:test:network-manager:request-handlers:request-array importing automatic-gas-handler.ts took 11ms
* 2025-03-18T10:54:37.576Z hardhat:test:network-manager:request-handlers:request-array importing automatic-gas-price-handler.ts took 11ms
* 2025-03-18T10:54:37.576Z hardhat:test:network-manager:request-handlers:request-array importing fixed-gas-handler.ts took 11ms
* 2025-03-18T10:54:37.576Z hardhat:test:network-manager:request-handlers:request-array importing automatic-sender-handler.ts took 10ms
* 2025-03-18T10:54:37.576Z hardhat:test:network-manager:request-handlers:request-array importing sender.ts took 10ms
* 2025-03-18T10:54:37.576Z hardhat:test:network-manager:request-handlers:request-array importing fixed-gas-price-handler.ts took 10ms
* 2025-03-18T10:54:37.576Z hardhat:test:network-manager:request-handlers:request-array importing multiplied-gas-estimation.ts took 10ms
*/

I'm leaving this PR in draft as it was useful to me during the investigation. I don't necesarilly want to merge it as I think it would be a pretty flaky addition. We can discuss the next steps either here or in the original issue.

Copy link

vercel bot commented Mar 18, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
hardhat ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 18, 2025 11:36am

Copy link

changeset-bot bot commented Mar 18, 2025

⚠️ No Changeset found

Latest commit: 4d429a1

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions github-actions bot added the status:ready This issue is ready to be worked on label Mar 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:ready This issue is ready to be worked on
Projects
Status: Backlog
Development

Successfully merging this pull request may close these issues.

1 participant