-
Notifications
You must be signed in to change notification settings - Fork 212
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
MXCrypto: Move decryptions out of the main thread #1091
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
4791c86
to
e15de19
Compare
ismailgulek
approved these changes
May 14, 2021
and make it the single object returned after a decryption
…ent id ex: to_device events
and ignore others. Do not consider them as errors
MXCrypto.handleRoomKeyEvent() has been added to know exactly when we can start decrypting.
…nt data It solves 2 things: - avoid synchronous decryption - decryption of the MXRoomSummary.lastMessageEvent when it is edited (element-hq/element-ios/issues/4322) Plus it saves a decryption.
to avoid useless (synchronous) decryption like in the previous commit
This will save synchronous decryption
to avoid any synchronous decryption. The algo is probably simpler. It also fixes testGetLastMessageFromPagination, testGetLastMessageFromSeveralPaginations and testFixRoomsSummariesLastMessage tests that have been broken since b4d5ba7
lastMessageEvent is required by room list. It took 1.3 seconds on the main thread to get it on my account (600 rooms). Now there are asynchronously preloaded and decrypted when the SDK load. One test has been fixed. As events are now decrypted only upstream and no more on demand, we need to trigger a decrypt to mimic this upstream decryption.
160a26a
to
2a87567
Compare
as it is not available in the decrypted JSON content
that does not pollute existing listeners on the live timeline There can be only one component that paginates message from a time else there is will be races.
Use MXSession.decryptEvents to get decrypted events.
…o decryptEvent:inTimeline:]
…ith the decryption queue. A single decryption takes 5ms on an iPhoneX. It was useless to synchronise this method with the busy decryption thread. There is no relationship. Note that this operation uses 0.5ms. It must not be called from the main thread. MatrixKit uses this method to build the timeline. It uses on a dedicated queue.
This was referenced May 19, 2021
MXKRoomDataSource: Decrypt unsent messages to follow MatrixSDK changes
matrix-org/matrix-ios-kit#815
Merged
We must not call complete() before returning. It breaks pagination in MatrixKit that uses the returned MXHTTPOperation.
This method is called from the main thread. It should not use directly elf.crypto.deviceList that is managed by the cryptoQueue.
Co-authored-by: SBiOSoftWhare <SBiOSoftWhare@users.noreply.github.com>
Co-authored-by: SBiOSoftWhare <SBiOSoftWhare@users.noreply.github.com>
End of the removal of decryption on the main thread
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix element-hq/element-ios#4306.
Fix element-hq/element-ios/issues/4322 in 6bfeee5
Events are now asynchronously decrypted upstream, where upstream can be:
This means we can remove latter calls of [MXSession decryptEvent:] that is synchronous and called mostly on the main thread.
On my account, an initial sync took 12.4 seconds on the main thread in the call of [MXSession decryptEvent:]. Now decryption takes 11s but on the decryption thread.
This change is more noticeable on incremental syncs, specially on the app restart, where decryption does not steal anymore time from the main thread (5 to 10 ms per decryption).
Note there is still a call of
[MXCrypto decryptEvent:]
forMXRoom.outgoingMessages
. It will be removed in a next PR.Commits can be reviewed one by one.