Skip to content

Commit

Permalink
Send over the new sourcetext's content checksum when sending notifica…
Browse files Browse the repository at this point in the history
…tion of textchanges (#75928)

* Send over the new sourcetext's content checksum when sending notification of textchanges

Calculation of this on the server was taking quite a bit of CPU (about 11.6% in the scrolling speedometer during the typing scenario). Instead, pass this data over as part of the text change notification, similar to what SerializableSourceText's serialization does.
  • Loading branch information
ToddGrun authored Nov 16, 2024
1 parent 5810b4f commit f6f0035
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 16 deletions.
5 changes: 3 additions & 2 deletions src/EditorFeatures/Core/Remote/SolutionChecksumUpdater.cs
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ private async ValueTask SynchronizeTextChangesAsync(
//
// otherwise, it will do the normal behavior of getting full text from VS side. this optimization saves
// times we need to do full text synchronization for typing scenario.
using var _ = ArrayBuilder<(DocumentId id, Checksum textChecksum, ImmutableArray<TextChange> changes)>.GetInstance(out var builder);
using var _ = ArrayBuilder<(DocumentId id, Checksum textChecksum, ImmutableArray<TextChange> changes, Checksum newTextChecksum)>.GetInstance(out var builder);

foreach (var (oldDocument, newDocument) in values)
{
Expand All @@ -242,9 +242,10 @@ private async ValueTask SynchronizeTextChangesAsync(
continue;

var state = await oldDocument.State.GetStateChecksumsAsync(cancellationToken).ConfigureAwait(false);
var newState = await newDocument.State.GetStateChecksumsAsync(cancellationToken).ConfigureAwait(false);

var textChanges = newText.GetTextChanges(oldText).AsImmutable();
builder.Add((oldDocument.Id, state.Text, textChanges));
builder.Add((oldDocument.Id, state.Text, textChanges, newState.Text));
}

if (builder.Count == 0)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,15 +89,15 @@ public async Task TestRemoteHostTextSynchronize()
// update text
var newText = oldText.WithChanges(new TextChange(TextSpan.FromBounds(0, 0), "/* test */"));

// sync
await client.TryInvokeAsync<IRemoteAssetSynchronizationService>(
(service, cancellationToken) => service.SynchronizeTextChangesAsync([(oldDocument.Id, oldState.Text, newText.GetTextChanges(oldText).AsImmutable())], cancellationToken),
CancellationToken.None);

// apply change to solution
var newDocument = oldDocument.WithText(newText);
var newState = await newDocument.State.GetStateChecksumsAsync(CancellationToken.None);

// sync
await client.TryInvokeAsync<IRemoteAssetSynchronizationService>(
(service, cancellationToken) => service.SynchronizeTextChangesAsync([(oldDocument.Id, oldState.Text, newText.GetTextChanges(oldText).AsImmutable(), newState.Text)], cancellationToken),
CancellationToken.None);

// check that text already exist in remote side
Assert.True(client.TestData.WorkspaceManager.SolutionAssetCache.TryGetAsset<SerializableSourceText>(newState.Text, out var serializableRemoteText));
Assert.Equal(newText.ToString(), (await serializableRemoteText.GetTextAsync(CancellationToken.None)).ToString());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,26 +57,31 @@ internal sealed class SerializableSourceText
public readonly Checksum ContentChecksum;

public SerializableSourceText(TemporaryStorageTextHandle storageHandle)
: this(storageHandle, text: null, storageHandle.ContentHash)
: this(storageHandle, text: null, Checksum.Create(storageHandle.ContentHash))
{
}

public SerializableSourceText(SourceText text, ImmutableArray<byte> contentHash)
: this(storageHandle: null, text, contentHash)
: this(storageHandle: null, text, Checksum.Create(contentHash))
{
}

private SerializableSourceText(TemporaryStorageTextHandle? storageHandle, SourceText? text, ImmutableArray<byte> contentHash)
public SerializableSourceText(SourceText text, Checksum contentChecksum)
: this(storageHandle: null, text, contentChecksum)
{
}

private SerializableSourceText(TemporaryStorageTextHandle? storageHandle, SourceText? text, Checksum contentChecksum)
{
Debug.Assert(storageHandle is null != text is null);

_storageHandle = storageHandle;
_text = text;
ContentChecksum = Checksum.Create(contentHash);
ContentChecksum = contentChecksum;

#if DEBUG
var computedContentHash = TryGetText()?.GetContentHash() ?? _storageHandle!.ContentHash;
Debug.Assert(contentHash.SequenceEqual(computedContentHash));
Debug.Assert(contentChecksum == Checksum.Create(computedContentHash));
#endif
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ internal interface IRemoteAssetSynchronizationService
/// <em>entire</em> contents of the file over.
/// </summary>
ValueTask SynchronizeTextChangesAsync(
ImmutableArray<(DocumentId documentId, Checksum baseTextChecksum, ImmutableArray<TextChange> textChanges)> changes,
ImmutableArray<(DocumentId documentId, Checksum baseTextChecksum, ImmutableArray<TextChange> textChanges, Checksum newTextChecksum)> changes,
CancellationToken cancellationToken);

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public ValueTask SynchronizeActiveDocumentAsync(DocumentId? documentId, Cancella
}

public ValueTask SynchronizeTextChangesAsync(
ImmutableArray<(DocumentId documentId, Checksum baseTextChecksum, ImmutableArray<TextChange> textChanges)> changes,
ImmutableArray<(DocumentId documentId, Checksum baseTextChecksum, ImmutableArray<TextChange> textChanges, Checksum newTextChecksum)> changes,
CancellationToken cancellationToken)
{
return RunServiceAsync(async cancellationToken =>
Expand All @@ -58,7 +58,7 @@ public ValueTask SynchronizeTextChangesAsync(
using (RoslynLogger.LogBlock(FunctionId.RemoteHostService_SynchronizeTextAsync, cancellationToken))
{
foreach (var (documentId, baseTextChecksum, textChanges) in changes)
foreach (var (documentId, baseTextChecksum, textChanges, newTextChecksum) in changes)
{
// Try to get the text associated with baseTextChecksum
var text = await TryGetSourceTextAsync(WorkspaceManager, workspace, documentId, baseTextChecksum, cancellationToken).ConfigureAwait(false);
Expand All @@ -73,7 +73,7 @@ public ValueTask SynchronizeTextChangesAsync(
// the asset cache so that future calls to retrieve it can do so quickly, without synchronizing over
// the entire document.
var newText = text.WithChanges(textChanges);
var newSerializableText = new SerializableSourceText(newText, newText.GetContentHash());
var newSerializableText = new SerializableSourceText(newText, newTextChecksum);
WorkspaceManager.SolutionAssetCache.GetOrAdd(newSerializableText.ContentChecksum, newSerializableText);
}
Expand Down

0 comments on commit f6f0035

Please sign in to comment.