Pull reading of last storage subsystem out of lock (part2) #73295
Conversation
dotnet-issue-labeler
bot
added
Area-IDE
untriaged
CyrusNajmabadi
changed the title
| private readonly CancellationTokenSource _shutdownTokenSource = new(); | ||
|
|
||
| private readonly string _solutionDirectory; | ||
|
|
||
| private readonly SQLiteConnectionPoolService _connectionPoolService; |
There was a problem hiding this comment.
inlined this type. it really wasn't needed as a separate singleton type.
| @@ -29,47 +29,17 @@ private async ValueTask FlushInMemoryDataToDiskIfNotShutdownAsync(CancellationTo | |||
| await PerformWriteAsync(_flushInMemoryDataToDisk, cancellationToken).ConfigureAwait(false); | |||
| } | |||
|
|
|||
| private Task FlushWritesOnCloseAsync() | |||
There was a problem hiding this comment.
this intentionally went away. There is an ABWQ that is already flushign writes to disk. That will still continue working up until the point that the disposal token fires (which means VS is shutting down). We've always had the semantics that if we are shutting down, we strictly shut down, potentially losing some writes to disk (note: at worse we only ever lose writes in the las 0.5 seconds before shutdown).
Tthis is always safe to then skip. If we have any final writes, they'll either get flushed or not. If they do, great. If they don't, that's fine as well. That's because the persistent cache is only ever considered 'best effort'. 100% of featues taht use it can only use it as a place they try to store data into, but work 100% fine if those writes didn't happen. This will be detected the next tmie VS runs when a feature tries to erad the data, and simply goes "the data isn't htere, so i'll compute it".
| ownershipLock.Dispose(); | ||
| } | ||
|
|
||
| throw; |
There was a problem hiding this comment.
so much complex disposable logic that goes away.
src/Workspaces/Core/Portable/Storage/AbstractPersistentStorageService.cs
Outdated
Show resolved
Hide resolved
|
@ToddGrun this is ready for review. |
src/Workspaces/Core/Portable/Storage/AbstractPersistentStorageService.cs
Outdated
Show resolved
Hide resolved
|
@jasonmalinowski For review when you get back. |
Updates storage system to no longer be disposable.