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

[composable-controller] perf: Optimize expensive reduce operations, fix metadata instantiation #4968

Draft
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

MajorLift
Copy link
Contributor

@MajorLift MajorLift commented Nov 25, 2024

Motivation

Expensive operations during ComposableController instantiation contribute to downstream performance issues in our client apps' initialization times. Optimizing these operations may be especially impactful since ComposableController takes large inputs consisting of all of the controllers used by a given client.

Explanation

  • Remove wasteful reduce operations that create temporary objects on each iteration and replace with implementations that re-use their output objects.
  • Refactor ComposableController to accept an object instead of an array. Now that the mobile Engine creates a controllers object, this saves us an extra operation for converting the object into an array.
    • For downstream application, see MetaMask/metamask-mobile@5ceb81c
    • An auxiliary benefit of this is slightly improved type safety. Since we were previously using a type array instead of a type tuple, we were vulnerable to duplicate entries in the controllers input. This is now resolved by using a type object.
  • Fixes issues with 'state', 'metadata' field instantiation (see changelogs).

References

Changelog

@metamask/composable-controller

Changed

  • BREAKING: ComposableController constructor option controllers and generic type argument ChildControllers are re-defined from an array of controller instances to an object that maps controller names to controller instances.
  • BREAKING: ComposableController 'state' field and 'metadata' objects exclude child controllers that do not extend from BaseController or BaseControllerV1.

Fixed

  • BREAKING: ComposableController 'metadata' field object now correctly populates BaseControllerV1 controller properties with a metadata object that maps state property names to StateMetadataProperty type objects.
    • Previously, BaseControllerV1 controllers were assigned the object { persist: true, anonymous: true }, and their state properties were overwritten.

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • I've highlighted breaking changes using the "BREAKING" category above as appropriate
  • I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes

@MajorLift MajorLift self-assigned this Nov 25, 2024
@MajorLift MajorLift added team-wallet-framework team-tiger Tiger team (for tech debt reduction + performance improvements) area-performance Issues relating to slowness of app, cpu usage, and/or blank screens. labels Nov 25, 2024
@MajorLift
Copy link
Contributor Author

@metamaskbot publish-preview

@MajorLift MajorLift marked this pull request as ready for review November 25, 2024 11:30
@MajorLift MajorLift requested a review from a team as a code owner November 25, 2024 11:30
Copy link
Contributor

Preview builds have been published. See these instructions for more information about preview builds.

Expand for full list of packages and versions.
{
  "@metamask-previews/accounts-controller": "20.0.0-preview-9d476a5",
  "@metamask-previews/address-book-controller": "6.0.1-preview-9d476a5",
  "@metamask-previews/announcement-controller": "7.0.1-preview-9d476a5",
  "@metamask-previews/approval-controller": "7.1.1-preview-9d476a5",
  "@metamask-previews/assets-controllers": "45.1.0-preview-9d476a5",
  "@metamask-previews/base-controller": "7.0.2-preview-9d476a5",
  "@metamask-previews/build-utils": "3.0.1-preview-9d476a5",
  "@metamask-previews/chain-controller": "0.2.0-preview-9d476a5",
  "@metamask-previews/composable-controller": "9.0.1-preview-9d476a5",
  "@metamask-previews/controller-utils": "11.4.3-preview-9d476a5",
  "@metamask-previews/ens-controller": "15.0.0-preview-9d476a5",
  "@metamask-previews/eth-json-rpc-provider": "4.1.6-preview-9d476a5",
  "@metamask-previews/gas-fee-controller": "22.0.1-preview-9d476a5",
  "@metamask-previews/json-rpc-engine": "10.0.1-preview-9d476a5",
  "@metamask-previews/json-rpc-middleware-stream": "8.0.5-preview-9d476a5",
  "@metamask-previews/keyring-controller": "19.0.0-preview-9d476a5",
  "@metamask-previews/logging-controller": "6.0.2-preview-9d476a5",
  "@metamask-previews/message-manager": "11.0.1-preview-9d476a5",
  "@metamask-previews/multichain": "1.0.0-preview-9d476a5",
  "@metamask-previews/name-controller": "8.0.1-preview-9d476a5",
  "@metamask-previews/network-controller": "22.0.2-preview-9d476a5",
  "@metamask-previews/notification-controller": "7.0.0-preview-9d476a5",
  "@metamask-previews/notification-services-controller": "0.14.0-preview-9d476a5",
  "@metamask-previews/permission-controller": "11.0.3-preview-9d476a5",
  "@metamask-previews/permission-log-controller": "3.0.1-preview-9d476a5",
  "@metamask-previews/phishing-controller": "12.3.0-preview-9d476a5",
  "@metamask-previews/polling-controller": "12.0.1-preview-9d476a5",
  "@metamask-previews/preferences-controller": "15.0.0-preview-9d476a5",
  "@metamask-previews/profile-sync-controller": "2.0.0-preview-9d476a5",
  "@metamask-previews/queued-request-controller": "7.0.1-preview-9d476a5",
  "@metamask-previews/rate-limit-controller": "6.0.1-preview-9d476a5",
  "@metamask-previews/selected-network-controller": "19.0.0-preview-9d476a5",
  "@metamask-previews/signature-controller": "23.0.0-preview-9d476a5",
  "@metamask-previews/transaction-controller": "40.0.0-preview-9d476a5",
  "@metamask-previews/user-operation-controller": "19.0.0-preview-9d476a5"
}

@MajorLift MajorLift marked this pull request as draft November 25, 2024 17:09
@MajorLift
Copy link
Contributor Author

@metamaskbot publish-preview

Copy link
Contributor

Preview builds have been published. See these instructions for more information about preview builds.

Expand for full list of packages and versions.
{
  "@metamask-previews/accounts-controller": "20.0.0-preview-aefb09ff",
  "@metamask-previews/address-book-controller": "6.0.1-preview-aefb09ff",
  "@metamask-previews/announcement-controller": "7.0.1-preview-aefb09ff",
  "@metamask-previews/approval-controller": "7.1.1-preview-aefb09ff",
  "@metamask-previews/assets-controllers": "45.1.0-preview-aefb09ff",
  "@metamask-previews/base-controller": "7.0.2-preview-aefb09ff",
  "@metamask-previews/build-utils": "3.0.1-preview-aefb09ff",
  "@metamask-previews/chain-controller": "0.2.0-preview-aefb09ff",
  "@metamask-previews/composable-controller": "9.0.1-preview-aefb09ff",
  "@metamask-previews/controller-utils": "11.4.3-preview-aefb09ff",
  "@metamask-previews/ens-controller": "15.0.0-preview-aefb09ff",
  "@metamask-previews/eth-json-rpc-provider": "4.1.6-preview-aefb09ff",
  "@metamask-previews/gas-fee-controller": "22.0.1-preview-aefb09ff",
  "@metamask-previews/json-rpc-engine": "10.0.1-preview-aefb09ff",
  "@metamask-previews/json-rpc-middleware-stream": "8.0.5-preview-aefb09ff",
  "@metamask-previews/keyring-controller": "19.0.0-preview-aefb09ff",
  "@metamask-previews/logging-controller": "6.0.2-preview-aefb09ff",
  "@metamask-previews/message-manager": "11.0.1-preview-aefb09ff",
  "@metamask-previews/multichain": "1.0.0-preview-aefb09ff",
  "@metamask-previews/name-controller": "8.0.1-preview-aefb09ff",
  "@metamask-previews/network-controller": "22.0.2-preview-aefb09ff",
  "@metamask-previews/notification-controller": "7.0.0-preview-aefb09ff",
  "@metamask-previews/notification-services-controller": "0.14.0-preview-aefb09ff",
  "@metamask-previews/permission-controller": "11.0.3-preview-aefb09ff",
  "@metamask-previews/permission-log-controller": "3.0.1-preview-aefb09ff",
  "@metamask-previews/phishing-controller": "12.3.0-preview-aefb09ff",
  "@metamask-previews/polling-controller": "12.0.1-preview-aefb09ff",
  "@metamask-previews/preferences-controller": "15.0.0-preview-aefb09ff",
  "@metamask-previews/profile-sync-controller": "2.0.0-preview-aefb09ff",
  "@metamask-previews/queued-request-controller": "7.0.1-preview-aefb09ff",
  "@metamask-previews/rate-limit-controller": "6.0.1-preview-aefb09ff",
  "@metamask-previews/selected-network-controller": "19.0.0-preview-aefb09ff",
  "@metamask-previews/signature-controller": "23.0.0-preview-aefb09ff",
  "@metamask-previews/transaction-controller": "40.0.0-preview-aefb09ff",
  "@metamask-previews/user-operation-controller": "19.0.0-preview-aefb09ff"
}

@MajorLift
Copy link
Contributor Author

@metamaskbot publish-preview

Copy link
Contributor

Preview builds have been published. See these instructions for more information about preview builds.

Expand for full list of packages and versions.
{
  "@metamask-previews/accounts-controller": "20.0.0-preview-9959ebb6",
  "@metamask-previews/address-book-controller": "6.0.1-preview-9959ebb6",
  "@metamask-previews/announcement-controller": "7.0.1-preview-9959ebb6",
  "@metamask-previews/approval-controller": "7.1.1-preview-9959ebb6",
  "@metamask-previews/assets-controllers": "45.1.0-preview-9959ebb6",
  "@metamask-previews/base-controller": "7.0.2-preview-9959ebb6",
  "@metamask-previews/build-utils": "3.0.1-preview-9959ebb6",
  "@metamask-previews/chain-controller": "0.2.0-preview-9959ebb6",
  "@metamask-previews/composable-controller": "9.0.1-preview-9959ebb6",
  "@metamask-previews/controller-utils": "11.4.3-preview-9959ebb6",
  "@metamask-previews/ens-controller": "15.0.0-preview-9959ebb6",
  "@metamask-previews/eth-json-rpc-provider": "4.1.6-preview-9959ebb6",
  "@metamask-previews/gas-fee-controller": "22.0.1-preview-9959ebb6",
  "@metamask-previews/json-rpc-engine": "10.0.1-preview-9959ebb6",
  "@metamask-previews/json-rpc-middleware-stream": "8.0.5-preview-9959ebb6",
  "@metamask-previews/keyring-controller": "19.0.0-preview-9959ebb6",
  "@metamask-previews/logging-controller": "6.0.2-preview-9959ebb6",
  "@metamask-previews/message-manager": "11.0.1-preview-9959ebb6",
  "@metamask-previews/multichain": "1.0.0-preview-9959ebb6",
  "@metamask-previews/name-controller": "8.0.1-preview-9959ebb6",
  "@metamask-previews/network-controller": "22.0.2-preview-9959ebb6",
  "@metamask-previews/notification-controller": "7.0.0-preview-9959ebb6",
  "@metamask-previews/notification-services-controller": "0.14.0-preview-9959ebb6",
  "@metamask-previews/permission-controller": "11.0.3-preview-9959ebb6",
  "@metamask-previews/permission-log-controller": "3.0.1-preview-9959ebb6",
  "@metamask-previews/phishing-controller": "12.3.0-preview-9959ebb6",
  "@metamask-previews/polling-controller": "12.0.1-preview-9959ebb6",
  "@metamask-previews/preferences-controller": "15.0.0-preview-9959ebb6",
  "@metamask-previews/profile-sync-controller": "2.0.0-preview-9959ebb6",
  "@metamask-previews/queued-request-controller": "7.0.1-preview-9959ebb6",
  "@metamask-previews/rate-limit-controller": "6.0.1-preview-9959ebb6",
  "@metamask-previews/selected-network-controller": "19.0.0-preview-9959ebb6",
  "@metamask-previews/signature-controller": "23.0.0-preview-9959ebb6",
  "@metamask-previews/transaction-controller": "40.0.0-preview-9959ebb6",
  "@metamask-previews/user-operation-controller": "19.0.0-preview-9959ebb6"
}

@MajorLift
Copy link
Contributor Author

@metamaskbot publish-preview

Copy link
Contributor

Preview builds have been published. See these instructions for more information about preview builds.

Expand for full list of packages and versions.
{
  "@metamask-previews/accounts-controller": "20.0.0-preview-ebca6967",
  "@metamask-previews/address-book-controller": "6.0.1-preview-ebca6967",
  "@metamask-previews/announcement-controller": "7.0.1-preview-ebca6967",
  "@metamask-previews/approval-controller": "7.1.1-preview-ebca6967",
  "@metamask-previews/assets-controllers": "45.1.0-preview-ebca6967",
  "@metamask-previews/base-controller": "7.0.2-preview-ebca6967",
  "@metamask-previews/build-utils": "3.0.1-preview-ebca6967",
  "@metamask-previews/chain-controller": "0.2.0-preview-ebca6967",
  "@metamask-previews/composable-controller": "9.0.1-preview-ebca6967",
  "@metamask-previews/controller-utils": "11.4.3-preview-ebca6967",
  "@metamask-previews/ens-controller": "15.0.0-preview-ebca6967",
  "@metamask-previews/eth-json-rpc-provider": "4.1.6-preview-ebca6967",
  "@metamask-previews/gas-fee-controller": "22.0.1-preview-ebca6967",
  "@metamask-previews/json-rpc-engine": "10.0.1-preview-ebca6967",
  "@metamask-previews/json-rpc-middleware-stream": "8.0.5-preview-ebca6967",
  "@metamask-previews/keyring-controller": "19.0.0-preview-ebca6967",
  "@metamask-previews/logging-controller": "6.0.2-preview-ebca6967",
  "@metamask-previews/message-manager": "11.0.1-preview-ebca6967",
  "@metamask-previews/multichain": "1.0.0-preview-ebca6967",
  "@metamask-previews/name-controller": "8.0.1-preview-ebca6967",
  "@metamask-previews/network-controller": "22.0.2-preview-ebca6967",
  "@metamask-previews/notification-controller": "7.0.0-preview-ebca6967",
  "@metamask-previews/notification-services-controller": "0.14.0-preview-ebca6967",
  "@metamask-previews/permission-controller": "11.0.3-preview-ebca6967",
  "@metamask-previews/permission-log-controller": "3.0.1-preview-ebca6967",
  "@metamask-previews/phishing-controller": "12.3.0-preview-ebca6967",
  "@metamask-previews/polling-controller": "12.0.1-preview-ebca6967",
  "@metamask-previews/preferences-controller": "15.0.0-preview-ebca6967",
  "@metamask-previews/profile-sync-controller": "2.0.0-preview-ebca6967",
  "@metamask-previews/queued-request-controller": "7.0.1-preview-ebca6967",
  "@metamask-previews/rate-limit-controller": "6.0.1-preview-ebca6967",
  "@metamask-previews/selected-network-controller": "19.0.0-preview-ebca6967",
  "@metamask-previews/signature-controller": "23.0.0-preview-ebca6967",
  "@metamask-previews/transaction-controller": "40.0.0-preview-ebca6967",
  "@metamask-previews/user-operation-controller": "19.0.0-preview-ebca6967"
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-performance Issues relating to slowness of app, cpu usage, and/or blank screens. team-tiger Tiger team (for tech debt reduction + performance improvements) team-wallet-framework
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant