Skip to content

[xcvrd] Optimize module initialization performance#611

Merged
prgeor merged 1 commit intosonic-net:masterfrom
Junchao-Mellanox:master-xcvrd-optimize
May 19, 2025
Merged

[xcvrd] Optimize module initialization performance#611
prgeor merged 1 commit intosonic-net:masterfrom
Junchao-Mellanox:master-xcvrd-optimize

Conversation

@Junchao-Mellanox
Copy link
Collaborator

@Junchao-Mellanox Junchao-Mellanox commented May 8, 2025

Description

  1. Use hget instead of hgetall to gain better performance
  2. Optimize SfpStateUpdateTask.init:
  • Use cache in this function for different logical ports with the same physical module.
  • Handles transceiver info table and media settings before any other table.

Motivation and Context

Improve module initialization performance. Due my test:

  1. hget is 7 times faster than hgetall when getting cmis module state
  2. Before optimizing SfpStateUpdateTask.init, the last module takes about 5 minutes to finish notify_media_settings call; after the optimization, the last module takes about 1 minutes to finish notify_media_settings` call.

How Has This Been Tested?

Manual test

Additional Information (Optional)

@mssonicbld
Copy link
Collaborator

/azp run

@Junchao-Mellanox Junchao-Mellanox marked this pull request as draft May 8, 2025 02:59
@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@Junchao-Mellanox Junchao-Mellanox force-pushed the master-xcvrd-optimize branch from d4413b7 to 474a0e7 Compare May 9, 2025 07:08
@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@Junchao-Mellanox Junchao-Mellanox requested review from keboliu, mihirpat1 and prgeor and removed request for mihirpat1 May 9, 2025 10:11
@Junchao-Mellanox Junchao-Mellanox force-pushed the master-xcvrd-optimize branch from 99bcda1 to d99a906 Compare May 9, 2025 10:19
@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@Junchao-Mellanox Junchao-Mellanox marked this pull request as ready for review May 9, 2025 10:26
@liat-grozovik
Copy link
Collaborator

@mihirpat1 @prgeor kindly hep to review and comment on this PR

Copy link
Collaborator

@prgeor prgeor left a comment

Choose a reason for hiding this comment

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

@mihirpat1 can you review?

Copy link
Collaborator

@prgeor prgeor left a comment

Choose a reason for hiding this comment

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

@Junchao-Mellanox please provide more details on the 2 optimization. What is the current perf number vs new with this change

@Junchao-Mellanox
Copy link
Collaborator Author

@Junchao-Mellanox please provide more details on the 2 optimization. What is the current perf number vs new with this change

Updated in description

@Junchao-Mellanox
Copy link
Collaborator Author

Hi @prgeor , could you please help merge?

@prgeor prgeor merged commit 45c1582 into sonic-net:master May 19, 2025
5 checks passed
@r12f
Copy link

r12f commented May 21, 2025

hi @Junchao-Mellanox , do you mind to help create a manual pick to 202412?

@Junchao-Mellanox
Copy link
Collaborator Author

hi @Junchao-Mellanox , do you mind to help create a manual pick to 202412?

PR ready: Azure/sonic-platform-daemons.msft#16

@r12f
Copy link

r12f commented May 22, 2025

thanks Junchao. It is merged now.

vvolam pushed a commit to vvolam/sonic-platform-daemons that referenced this pull request Jun 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants