feat(plugin-meetings): optimize the order of data sets initialization#4821
feat(plugin-meetings): optimize the order of data sets initialization#4821marcin-bazyl merged 6 commits intowebex:nextfrom
Conversation
|
This pull request is automatically being deployed by Amplify Hosting (learn more). |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 401a519eca
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const updatedObjects: HashTreeObject[] = []; | ||
|
|
||
| for (const dataSet of visibleDataSets) { | ||
| for (const dataSet of sortByInitPriority(visibleDataSets)) { |
There was a problem hiding this comment.
Restore main-before-self order for initialization updates
Sorting visibleDataSets with sortByInitPriority makes self initialization run before main, which changes the order of updatedObjects emitted from initial sync. On first hash-tree bootstrap, LocusInfo.updateFromHashTree starts from an empty locus, and updateLocusFromHashTreeObject only creates a webinar attendee participant from self when locus.info?.isWebinar is already present (see packages/@webex/plugin-meetings/src/locus-info/index.ts, ObjectType.self handling). Processing self before main means that condition is false, so the self participant is skipped for that batch and downstream code that expects it can misbehave.
Useful? React with 👍 / 👎.
|
|
||
| // Send a message that adds two new visible datasets: 'new-main' and 'new-self' | ||
| // where neither has info in dataSets (so both require async initialization) | ||
| // Note: we use names that match priority names to test ordering |
There was a problem hiding this comment.
I don't understand this bit - the comment says we're using names that match the priority names, but they don't match - "main-2" and "self-2" doesn't match "main" or "self", so I think they just happen to get initialized in the order that they are in the message. can we change this test so that we start without main or self and add one of them in the message together with some other datasets and check that it gets initialized first?
Thank you for your contribution! |
COMPLETES #< INSERT LINK TO ISSUE >
https://jira-eng-gpk2.cisco.com/jira/browse/SPARK-779086
This pull request addresses
Prioritize initializing the most important data sets first "main" and "self"
< DESCRIBE THE CONTEXT OF THE ISSUE >
by making the following changes
< DESCRIBE YOUR CHANGES >
Change Type
The following scenarios were tested
< ENUMERATE TESTS PERFORMED, WHETHER MANUAL OR AUTOMATED >
The GAI Coding Policy And Copyright Annotation Best Practices
I certified that
Make sure to have followed the contributing guidelines before submitting.