Skip to content

Commit faecb85

Browse files
committed
Answer an attached viewer's unchanged listing without resending it
An attached viewer asks for the full listing five times a second, even hidden, and a full listing is every patch base64'd twice by the owner, inside its gate, then parsed again by the viewer - to learn, almost every time, that nothing changed. The owner now tags each full listing, and the viewer sends back the tag of the one it holds. While the tag still stands the owner answers "unchanged" and nothing else, and the viewer keeps the queue it already parsed; the tracked files are still stat'd each pass, since a re-run rewriting one changes no listing. The tag is taken before the listing is built, so a change landing in between costs one more full listing rather than a stale one kept. A tag is cheap to take: both queues are immutable, so a new reference is a new generation. The tray also folds in its tracked files, which its own scan changes, and a count of stashed window commands, so a focus is never answered unchanged. Each owner's tags carry an id of their own, so a tag held across an owner restart cannot match. A hidden window lists once a second instead of five times. Older owners ignore the tag and answer in full; older viewers send none and are answered in full.
1 parent 1135222 commit faecb85

15 files changed

Lines changed: 338 additions & 10 deletions

‎src/DiffEngine.Tests/InlineQueueClientTests.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -404,6 +404,8 @@ ViewerResponse IQueueOwner.Listing(bool withPatches)
404404
}
405405
}
406406

407+
string? IQueueOwner.ListingTag() => null;
408+
407409
bool IQueueOwner.Has(string key)
408410
{
409411
lock (gate)

‎src/DiffEngine.Tests/ViewerProtocolTests.cs‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -479,6 +479,49 @@ static ViewerResponse RoundTrip(ViewerResponse response)
479479
await Assert.That(RoundTrip(ViewerResponse.Success("Applied Tests.cs:42")).Written).IsNull();
480480
}
481481

482+
/// <summary>
483+
/// An attached viewer asks for the full listing five times a second, and it is every patch
484+
/// serialized. Sent the tag of the one it holds, an owner whose queue has not changed since
485+
/// answers that it has not, with nothing else. An owner with no tag to give, and a reader
486+
/// sending none, get the listing in full as before tags.
487+
/// </summary>
488+
[Test]
489+
public async Task AnUnchangedListingIsNotSentAgain()
490+
{
491+
static ViewerResponse RoundTrip(ViewerResponse response)
492+
{
493+
if (!ViewerResponse.TryParse(response.Build(), out var parsed))
494+
{
495+
throw new("Unreadable response.");
496+
}
497+
498+
return parsed;
499+
}
500+
501+
var owner = new FakeOwner((true, null))
502+
{
503+
Tag = "one"
504+
};
505+
506+
var full = RoundTrip(ViewerMessageHandler.Handle(owner, new(ViewerVerb.ListFull)));
507+
await Assert.That(full.Tag).IsEqualTo("one");
508+
await Assert.That(full.Unchanged).IsFalse();
509+
510+
var same = RoundTrip(ViewerMessageHandler.Handle(owner, new(ViewerVerb.ListFull, Body: "one")));
511+
await Assert.That(same.Unchanged).IsTrue();
512+
await Assert.That(same.Tag).IsEqualTo("one");
513+
514+
owner.Tag = "two";
515+
var changed = RoundTrip(ViewerMessageHandler.Handle(owner, new(ViewerVerb.ListFull, Body: "one")));
516+
await Assert.That(changed.Unchanged).IsFalse();
517+
await Assert.That(changed.Tag).IsEqualTo("two");
518+
519+
owner.Tag = null;
520+
var untagged = RoundTrip(ViewerMessageHandler.Handle(owner, new(ViewerVerb.ListFull, Body: "two")));
521+
await Assert.That(untagged.Unchanged).IsFalse();
522+
await Assert.That(untagged.Tag).IsNull();
523+
}
524+
482525
/// <summary>
483526
/// A pending file with no tray running. The paths ride key and body rather than an encoded
484527
/// payload, because that is all a tracked move or delete is.
@@ -592,6 +635,10 @@ public void TrackDelete(string file) =>
592635

593636
public ViewerResponse Listing(bool withPatches) => ViewerResponse.Listing([]);
594637

638+
public string? Tag { get; set; }
639+
640+
public string? ListingTag() => Tag;
641+
595642
public bool Has(string key) => true;
596643

597644
public (bool ok, string? message, bool written) Accept(string key, string? origin)

‎src/DiffEngine/Protocol/IQueueOwner.cs‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,14 @@ interface IQueueOwner
4242
/// </summary>
4343
ViewerResponse Listing(bool withPatches);
4444

45+
/// <summary>
46+
/// Something that changes whenever the full listing would, and is cheap to take: the listing
47+
/// is every patch serialized, and an attached viewer asks for it five times a second. Unique
48+
/// to this owner, so a tag from an owner that has since restarted can never match. Null to
49+
/// always answer in full.
50+
/// </summary>
51+
string? ListingTag();
52+
4553
bool Has(string key);
4654

4755
/// <summary>

‎src/DiffEngine/Protocol/ViewerMessageHandler.cs‎

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ public static ViewerResponse Handle(IQueueOwner owner, ViewerMessage message)
2626
case ViewerVerb.List:
2727
return owner.Listing(false);
2828
case ViewerVerb.ListFull:
29-
return owner.Listing(true);
29+
return ListFull(owner, message.Body);
3030
case ViewerVerb.Accept:
3131
case ViewerVerb.Discard:
3232
return Act(owner, message.Key, message.Body, message.Verb);
@@ -134,6 +134,28 @@ static ViewerResponse Delete(IQueueOwner owner, string? file)
134134
return ViewerResponse.Success();
135135
}
136136

137+
/// <summary>
138+
/// The body is the tag of the listing the reader already holds, if any. A reader that predates
139+
/// tags sends none and gets the listing in full, as ever.
140+
/// </summary>
141+
static ViewerResponse ListFull(IQueueOwner owner, string? since)
142+
{
143+
// Taken before the listing is built and never after. A change landing in between then
144+
// costs one more full listing next time, where the other order sent a listing older than
145+
// the tag it went under, and the reader would have kept it until something else changed.
146+
var tag = owner.ListingTag();
147+
if (tag is not null &&
148+
tag == since)
149+
{
150+
return ViewerResponse.UnchangedSince(tag);
151+
}
152+
153+
return owner.Listing(true) with
154+
{
155+
Tag = tag
156+
};
157+
}
158+
137159
static ViewerResponse Act(IQueueOwner owner, string? key, string? body, ViewerVerb verb)
138160
{
139161
if (key is null)

‎src/DiffEngine/Protocol/ViewerResponse.cs‎

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,28 @@ record ViewerResponse(
7979
/// </summary>
8080
public bool? Written { get; init; }
8181

82+
/// <summary>
83+
/// On a full listing: what the owner's queue looked like when it was taken, for the reader to
84+
/// send back on its next <see cref="ViewerVerb.ListFull"/>. Opaque, and only ever compared for
85+
/// equality by the owner that made it. Null from an owner that predates it.
86+
/// </summary>
87+
public string? Tag { get; init; }
88+
89+
/// <summary>
90+
/// The answer to a full listing whose tag still stands: nothing has changed, so nothing is
91+
/// sent, and the reader keeps what it has. An attached viewer asks five times a second, and a
92+
/// full listing is every patch base64'd twice by the owner, inside its gate, and parsed again
93+
/// by the reader.
94+
/// </summary>
95+
public bool Unchanged { get; init; }
96+
97+
public static ViewerResponse UnchangedSince(string tag) =>
98+
new(true, null, [])
99+
{
100+
Tag = tag,
101+
Unchanged = true
102+
};
103+
82104
public static ViewerResponse Success(string? message = null) =>
83105
new(true, message, []);
84106

@@ -123,6 +145,12 @@ public string Build()
123145
builder.Append($"written: {(written ? "true" : "false")}\n");
124146
}
125147

148+
ViewerPayload.Append(builder, "tag", Tag);
149+
if (Unchanged)
150+
{
151+
builder.Append("unchanged: true\n");
152+
}
153+
126154
foreach (var item in Items)
127155
{
128156
var status = item.Status is null ? "" : ViewerPayload.Encode(item.Status);
@@ -172,6 +200,8 @@ public static bool TryParse(string text, [NotNullWhen(true)] out ViewerResponse?
172200
string? windowKey = null;
173201
AcceptProgress? progress = null;
174202
bool? written = null;
203+
string? tag = null;
204+
var unchanged = false;
175205
var items = new List<ViewerResponseItem>();
176206
var moves = new List<ViewerResponseMove>();
177207
var deletes = new List<ViewerResponseDelete>();
@@ -208,6 +238,16 @@ public static bool TryParse(string text, [NotNullWhen(true)] out ViewerResponse?
208238
case "written":
209239
written = value == "true";
210240
continue;
241+
case "tag":
242+
if (!ViewerPayload.TryDecode(value, out tag))
243+
{
244+
return false;
245+
}
246+
247+
continue;
248+
case "unchanged":
249+
unchanged = value == "true";
250+
continue;
211251
case "message":
212252
if (!ViewerPayload.TryDecode(value, out message))
213253
{
@@ -289,7 +329,9 @@ public static bool TryParse(string text, [NotNullWhen(true)] out ViewerResponse?
289329
Moves = moves,
290330
Deletes = deletes,
291331
Progress = progress,
292-
Written = written
332+
Written = written,
333+
Tag = tag,
334+
Unchanged = unchanged
293335
};
294336
return true;
295337
}

‎src/DiffEngineTray.Tests/TrayViewerSyncTest.cs‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -281,6 +281,33 @@ public async Task AViewerGroupAcceptCarriesOutItsDeletesOnceTheTrayWroteTheSnaps
281281
await Assert.That(await File.ReadAllTextAsync(move.Target)).IsEqualTo("received");
282282
}
283283

284+
/// <summary>
285+
/// The tray answers an unchanged listing without it, but its tracked files change on their own
286+
/// scan rather than through the queue, and a focus it has stashed only reaches a window on a
287+
/// listing. Neither may be taken for unchanged.
288+
/// </summary>
289+
[Test]
290+
public async Task ATrackedFileOrAStashedFocusIsNeverAnsweredUnchanged()
291+
{
292+
await using var pair = new TrayOwned();
293+
var snapshot = pair.Queue(sample, 1);
294+
var tag = pair.Send(new(ViewerVerb.ListFull)).Tag;
295+
await Assert.That(pair.Send(new(ViewerVerb.ListFull, Body: tag)).Unchanged).IsTrue();
296+
297+
pair.AddDelete();
298+
var withDelete = pair.Send(new(ViewerVerb.ListFull, Body: tag));
299+
await Assert.That(withDelete.Unchanged).IsFalse();
300+
await Assert.That(withDelete.Deletes).HasSingleItem();
301+
302+
pair.Host.Focus(snapshot);
303+
var focused = pair.Send(new(ViewerVerb.ListFull, Body: withDelete.Tag));
304+
await Assert.That(focused.Unchanged).IsFalse();
305+
await Assert.That(focused.Window).IsEqualTo(WindowCommand.Focus);
306+
307+
// Taken by that listing, so the next is unchanged again
308+
await Assert.That(pair.Send(new(ViewerVerb.ListFull, Body: focused.Tag)).Unchanged).IsTrue();
309+
}
310+
284311
[Test]
285312
public async Task ViewerDiscardOfOneSnapshotReachesTheTray()
286313
{

‎src/DiffEngineTray/OwnedInlineHost.cs‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,16 @@ sealed class OwnedInlineHost :
4141
/// </summary>
4242
AcceptProgress? progress;
4343

44+
/// <summary>
45+
/// What <see cref="IQueueOwner.ListingTag"/> is made of, under <see cref="gate"/>. The queue
46+
/// is immutable and replaced on every change, so a new reference is a new generation; asks
47+
/// counts window commands stashed, so a listing carrying one is never answered unchanged.
48+
/// </summary>
49+
readonly string instance = Guid.NewGuid().ToString("N");
50+
InlineQueue? taggedQueue;
51+
long generation;
52+
long asks;
53+
4454
/// <summary>
4555
/// One accept-all at a time. The menu, a hot key and a displaying viewer can each ask for one,
4656
/// and two sweeping the same queue at once would apply every snapshot twice and report two
@@ -284,6 +294,40 @@ ViewerResponse IQueueOwner.Listing(bool withPatches)
284294
}
285295
}
286296

297+
string IQueueOwner.ListingTag()
298+
{
299+
// The tracked files by what the listing would carry of them rather than by a count, since
300+
// the tracker changes them on its own scan as well as through here. A handful of paths,
301+
// where the listing it saves is every patch.
302+
var files = TrackedFiles is { } tracked ? Fingerprint(tracked) : "";
303+
lock (gate)
304+
{
305+
if (!ReferenceEquals(queue, taggedQueue))
306+
{
307+
taggedQueue = queue;
308+
generation++;
309+
}
310+
311+
return $"{instance}.{generation}.{asks}.{progress?.Build()}.{files}";
312+
}
313+
}
314+
315+
static string Fingerprint(ITrackedFiles tracked)
316+
{
317+
var builder = new StringBuilder();
318+
foreach (var move in tracked.Moves())
319+
{
320+
builder.Append(move).Append('\n');
321+
}
322+
323+
foreach (var delete in tracked.Deletes())
324+
{
325+
builder.Append(delete).Append('\n');
326+
}
327+
328+
return Convert.ToHexString(System.Security.Cryptography.SHA256.HashData(Encoding.UTF8.GetBytes(builder.ToString())));
329+
}
330+
287331
bool IQueueOwner.Has(string key)
288332
{
289333
if (TrackedKeys.IsTracked(key))
@@ -650,6 +694,7 @@ void Ask(WindowCommand command, string? key)
650694
{
651695
window = command;
652696
windowKey = key;
697+
asks++;
653698
}
654699
}
655700

‎src/DiffEngineViewer.Tests/AttachedViewerTests.cs‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,70 @@ await Assert.That(queue.Select(_ => _.Name))
3636
await Assert.That(queue[0].TotalRows).IsGreaterThan(0);
3737
}
3838

39+
/// <summary>
40+
/// Asked with the tag of the listing it gave last, an owner whose queue has not moved answers
41+
/// that and nothing else, rather than every patch again. Any change makes it answer in full.
42+
/// </summary>
43+
[Test]
44+
public async Task AnUnchangedQueueIsNotSentAgain()
45+
{
46+
using var owner = new ServerFixture();
47+
owner.Send(Inline(Fixtures.Patch()));
48+
49+
var full = owner.Send(new(ViewerVerb.ListFull));
50+
await Assert.That(full.Tag).IsNotNull();
51+
await Assert.That(full.Items).HasSingleItem();
52+
53+
var same = owner.Send(new(ViewerVerb.ListFull, Body: full.Tag));
54+
await Assert.That(same.Unchanged).IsTrue();
55+
await Assert.That(same.Items).IsEmpty();
56+
57+
owner.Send(Inline(Fixtures.Patch("OtherTests.cs", 7, null, "new")));
58+
var changed = owner.Send(new(ViewerVerb.ListFull, Body: full.Tag));
59+
await Assert.That(changed.Unchanged).IsFalse();
60+
await Assert.That(changed.Items.Count).IsEqualTo(2);
61+
}
62+
63+
/// <summary>
64+
/// What an unchanged answer stands for is the listing held from before it, so the window keeps
65+
/// showing it, and still follows the owner once something does change.
66+
/// </summary>
67+
[Test]
68+
public async Task AnAttachedViewerFollowsAChangeAfterAnUnchangedListing()
69+
{
70+
using var owner = new ServerFixture();
71+
owner.Send(Inline(Fixtures.Patch()));
72+
var (host, link) = Attach(owner);
73+
link.Pump();
74+
var shown = host.State;
75+
76+
link.Pump();
77+
await Assert.That(host.State).IsSameReferenceAs(shown);
78+
79+
owner.Send(Inline(Fixtures.Patch("OtherTests.cs", 7, null, "new")));
80+
link.Pump();
81+
await Assert.That(host.State.Queue.Select(_ => _.Name))
82+
.IsEquivalentTo(["SampleTests.cs:42", "OtherTests.cs:7"]);
83+
}
84+
85+
/// <summary>
86+
/// A send that changes nothing still says so, on a listing that was answered unchanged.
87+
/// </summary>
88+
[Test]
89+
public async Task ARefusalArrivesOnAnUnchangedListing()
90+
{
91+
using var owner = new ServerFixture();
92+
owner.Send(Inline(Fixtures.Patch()));
93+
var (host, link) = Attach(owner);
94+
link.Pump();
95+
96+
link.Post(ViewerVerb.Discard, "nothing|1");
97+
link.Pump();
98+
99+
await Assert.That(host.State.Message).IsEqualTo("No pending snapshot for nothing|1");
100+
await Assert.That(host.State.Queue).HasSingleItem();
101+
}
102+
39103
[Test]
40104
public async Task AFailedEntryKeepsItsStatus()
41105
{
Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
{
22
Ok: false,
3-
Message: No pending snapshot for missing|1
3+
Message: No pending snapshot for missing|1,
4+
Unchanged: false
45
}

‎src/DiffEngineViewer.Tests/IpcTests.List.verified.txt‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,5 +9,6 @@
99
Key: othertests.cs|7,
1010
Name: othertests.cs:7
1111
}
12-
]
12+
],
13+
Unchanged: false
1314
}

0 commit comments

Comments
 (0)