[PM-3426] Make cipher decryption faster by running in parallel

[quexten]

Type of change

• Bug fix
• New feature development
• Tech debt (refactoring, code cleanup, dependency upgrades, etc)
• Build/deploy pipeline (DevOps)
• Other

Objective

With a lot of ciphers in the vault, opening the vault (unlock/login) takes quite long (~25 seconds for my 10k cipher test vault). One of the causes seems to be that the cipher decryption runs concurrently (through async await) but not in parallel.

This PR rewrites the cipher decryption logic a bit to be both more readable and to run in parallel. On my emulator device, this brought the decryption time down to ~15 seconds. (An improvement, but there might still be a bottleneck somewhere else, since not all CPU resources were fully utilized. On the other hand, the emulator might also hinder further parallelization).

Related:
bitwarden/mobile#579

Code changes

• CipherService.cs use linq .AsParallel() to run the decryption in parallel

Screenshots

Before you submit

• Please check for formatting errors (dotnet format --verify-no-changes) (required)
• Please add unit tests where it makes sense to do so (encouraged but not required)
• If this change requires a documentation update - notify the documentation team
• If this change has particular deployment requirements - notify the DevOps team

[bitwarden-bot]

Thank you for your contribution! We've added this to our internal Community PR board for review.
ID: PM-3426

[vvolkgang]

Good catch! Added one small note about the usage of .Result but, I believe we might have some additional quick wins here.

Before we get into that, we should take the opportunity and write a repeatable-ish performance test so we can have more confidence in our findings. For the time being this will probably be a new method / class in our Core.Tests and we can later look at an in-app test.

Checking with our devs and I'll circle back!

[vvolkgang]

While I understand why the current approach led you here, we need to go Async All The Way, ie, not using .Result.

It's usually a great indicator that everything else needs to be refactored.

[quexten]

Thanks for the article. I'm not too familiar with how c# handles async. I believe in this case it might not have been a problem, since every synchronous, blocking .Result is on a separate thread and thus has a different synchronization context, but I might also be wrong about this.

Either way, I've changed it now to use async only, so it should be fine now.

[quexten]

To get some more accurate numbers to this, I measured the time in emulator on my 10k cipher test account (using DateTime.Now). I think the numbers paint a pretty clear picture.

Time	Device Name	Type	PID	Tag	Message
08-18 23:06:32.668	New_Device_1_API_34	Verbose	17698	mono-stdout	Time taken new 11.434737
08-18 23:06:18.895	New_Device_1_API_34	Verbose	17698	mono-stdout	Time taken old 33.309191

(This is specifically only decryption time, getting the encrypted ciphers from storage using await GetAllAsync(); (which is not parallel) was excluded.)

This is a ~200% speedup in emulator, likely more on actual devices.

[quexten]

Closing. The native apps make this irrelevant.