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

Investigate startup time regression #380

Closed
Gnuxie opened this issue May 3, 2024 · 3 comments
Closed

Investigate startup time regression #380

Gnuxie opened this issue May 3, 2024 · 3 comments
Labels
blocks release used in exceptional circumstances when the `latest` tag would break L4 Most Users Likelihood needs investigation P4 Blocks All Development Priority level - breaking build / dependency / test T5 Major usability Impairs usability in key scenarios.

Comments

@Gnuxie
Copy link
Member

Gnuxie commented May 3, 2024

At startup we changed the code in MPS to load rooms from the backing store all at once rather than one by one

https://github.com/Gnuxie/matrix-protection-suite/blob/6ae7c8ce8226cb18eea941345d9907d62aa75b5d/src/Protection/ProtectedRoomsManager/StandardProtectedRoomsManager.ts#L103

https://github.com/Gnuxie/matrix-protection-suite/blob/c3e574a1f7740e5c8057fbf43e9c98d9bd32e640/src/StateTracking/StandardSetRoomState.ts#L50-L52

-- or so i thought, but they should in effect be exactly the same? Either way on breadpirates.chat i noticed startup going from a 10seconds to ~1minute after this change, which is about as long as it takes without the backing store... so not sure what is going on.

The difference between main and v2.0.0-beta.2 looks like this:
image

I suspect that the loading code is a red herring, and in reality the changes are due to changes to typebox and the schema in the same release of MPS

@Gnuxie Gnuxie added L4 Most Users Likelihood P4 Blocks All Development Priority level - breaking build / dependency / test T5 Major usability Impairs usability in key scenarios. needs investigation blocks release used in exceptional circumstances when the `latest` tag would break labels May 4, 2024
Gnuxie added a commit to Gnuxie/matrix-protection-suite that referenced this issue May 4, 2024
We believe this is the cause of a performance regression in Draupnir:
the-draupnir-project/Draupnir#380.
Unfortunately we don't have the infrastructure in our unit testing
or deployment to be able to truthfully understand the cause.

So we have created a follow up the-draupnir-project/Draupnir#389.
Our approach for now is just to guess some of the most glaring
changes that have caused the regression.

If this fails to make a significant difference, other culprits
are changes to the parsing code (ie, there was a recent version
of MPS where we moved away from accidentally using the empty
schema for most event content), including the recent upgrade
to typebox 0.32.22.
Gnuxie added a commit that referenced this issue May 4, 2024
This contains a workaround for this issue #380
please see Gnuxie/matrix-protection-suite@e57f266.
Gnuxie added a commit that referenced this issue May 4, 2024
This contains a workaround for this issue #380
please see Gnuxie/matrix-protection-suite@e57f266.
@Gnuxie
Copy link
Member Author

Gnuxie commented May 4, 2024

Ok workaround didn't work.

@Gnuxie Gnuxie changed the title Investigate why the backing store takes a huge penalty from concurrent calls when it is supposedly synchronous Investigate startup time regression May 5, 2024
@Gnuxie
Copy link
Member Author

Gnuxie commented May 5, 2024

Gnuxie added a commit to Gnuxie/matrix-protection-suite that referenced this issue May 6, 2024
Gnuxie added a commit to Gnuxie/matrix-protection-suite that referenced this issue May 6, 2024
Gnuxie added a commit that referenced this issue May 6, 2024
Part of #380.
We've updated typebox and changed the way events are decoded.
Gnuxie added a commit that referenced this issue May 6, 2024
Part of #380.
We've updated typebox and changed the way events are decoded.
@Gnuxie
Copy link
Member Author

Gnuxie commented May 6, 2024

image

@Gnuxie Gnuxie closed this as completed May 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocks release used in exceptional circumstances when the `latest` tag would break L4 Most Users Likelihood needs investigation P4 Blocks All Development Priority level - breaking build / dependency / test T5 Major usability Impairs usability in key scenarios.
Projects
Status: No status
Development

No branches or pull requests

1 participant