Defer loading of SQLite binaries until they will be used #26269
Conversation
| @@ -88,6 +88,13 @@ protected override string GetDatabaseFilePath(string workingFolderPath) | |||
| protected override bool TryOpenDatabase( | |||
| Solution solution, string workingFolderPath, string databaseFilePath, out IPersistentStorage storage) | |||
| { | |||
| if (!TryInitializeLibraries()) | |||
There was a problem hiding this comment.
hey. turns out not that bad!
sharwell
changed the title
can you give info on what sort of impact this was? I'm surprised RPS didn't complain if this does impact things. |
|
@CyrusNajmabadi There was no measured impact. This is a generic statement on the basis for RPS checking assembly loads. |
|
Ah, i see. That matches my recollection as well that this was paid for by not having esent. But, i'm totally for deferring unnecessary work. I just misinterpretted the statements above. Thanks for the clarification! |
|
Do you think there's any way to write a test for this? It seems super simple to accidentally regress this sort of thing. |
|
@CyrusNajmabadi RPS is the regression test here |
|
I just meant something faster than that for the team. something we could catch at CI time versus potentially months later as builds RI and whatnot. |
|
Thanks for addressing! |
jinujoseph
added
Needs Shiproom Approval
Approved to merge
and removed
Needs Shiproom Approval
labels
|
sorry wrong button |
Fixes https://devdiv.visualstudio.com/DevDiv/_workitems/edit/594591
Customer scenario
When opening a small solution, SQLite assemblies are loaded by Visual Studio even though they are not used. In aggregate, assembly loads like this cause delays for users trying to simply open their solution for editing.¹
¹ We don't have evidence of the assemblies here being a direct problem, but this is a general explanation for why we look for the total number of assembly loads as part of the product performance regression test suite.
Bugs this fixes
https://devdiv.visualstudio.com/DevDiv/_workitems/edit/594591
Workarounds, if any
None needed (no functional/behavioral change).
Risk
Low. The initialization code is the same as before and only runs once, but is relocated to follow some fast-path checks for small solutions.
Performance impact
Improves performance when opening solutions that do not exceed the solution size threshold for persisting data.
Is this a regression from a previous update?
Yes. Introduced by #25590.
Root cause analysis
Caught by RPS.
How was the bug found?
RPS.
Test documentation updated?
N/A