Fix the bugs and the performance items from the review: the viewer's model, the library, the inline patcher, the tray and the three heads - #922
Merged
Conversation
…nts that stopped being read The five items under Bugs in todo.md, each with tests that fail without the fix. - The viewer no longer asks for a verified file. It was declared RequiresTarget, so for a snapshot with no verified file EmptyFiles wrote a placeholder before the viewer heard of the pair, and the viewer compared against it as though it were the expected file: a blank page, or a 212 byte PDF it could not open, which also hid the received text. EmptyFiles has no file for .geojson, .gpx, .kml, .topojson, .wkt, .wkb, .fgb or .geoparquet, so for those the launch ended at NoEmptyFileForExtension and nothing was raised at all. The viewer reads a missing target as the empty side of a new snapshot, which is what it now gets. - A pair that arrives again saying what the queued one says leaves the reader where they are, as an inline patch already did. A test that keeps failing the same way sends its pair on every run, and each one opened the entry again: back at its first change and first page, fitted, with the menu closed. The entry is replaced by one that reads the same and carries the new stamps, never kept as the same object, because TrackedWatch applies what a pass found by reference and a pass that looked while the run had cleared its received file found it gone. A document is compared by its bytes, since its text is read after it arrives and the arrival may not have it yet. - DocumentWatch's loop no longer ends on a fault. A copy that could not be written into the cache, because something held the file or the disk was full, threw out of the pass, the loop said so once and returned, and no document was read or drawn again until the viewer was restarted. A turn that fails now says why, once for as long as that stays the reason, and is tried again, and the status line is given back when it gets through. Nothing is marked as started before everything that can fail has been done, so a drawing whose copy could not be written is not left as a spinner. A file that could not be read for a moment is tried again too, rather than recorded as changed. - A PDF left behind after the timeout no longer disables PDFs for the life of the window. The flag is cleared when the call that was left behind returns, and a PDF opened meanwhile waits for it and is then read, with nothing recorded against it. The timeout is counted from the last page to land rather than from the start, so a long document that keeps landing pages is not given up on part way through. - A setting one viewer put back to its default is no longer restored by another viewer's next write. A default is stored as no line at all, and each write laid everything this viewer held back over the file wherever the file had no value, so the projection one window reset came back with the other's window position. A write is now the file as it stands with the one key changed, plus any of this viewer's own whose earlier write failed.
The review this list came from was run with six others beside it, one each for the library, the inline patcher, the tray, and the Windows, Linux and macOS heads. Their findings arrived after the first list was written and are added here, unfixed, by area. Each says how far it was taken. Four inline patcher findings were reproduced by running the patcher, and the F# one by compiling what it writes with dotnet fsi. Five more were confirmed by reading the code they rest on. The rest are marked as reported, with the reviewer's own evidence, after checking that the code each one quotes is in the tree as quoted. Nothing under a Linux or macOS heading has been run.
…ow's place, and a picture composed for ever - A decode that lands for a picture no longer on screen is thrown away, and it left the path marked as on its way. A reader who came back to that picture had nothing started for it, and a spinner in each pane until the file changed: stepping past a picture before it had decoded, which holding Tab through a queue of them does to nearly every one. The mark now goes when the decode lands, whatever is done with the result. - The first window was centred for its unscaled size and then scaled in place. WinForms centres as it creates the window, before OnHandleCreated sizes it for the display, so it grew down and to the right from there: at 150% on a 1080p display the footer was under the taskbar, and that was the placement remembered for every run after. It is centred again for the size it has. - Raise put a minimised window back to normal whatever it had been minimised from, so a maximised window came back at its restored size and was then remembered as not maximised. It goes back to what it was. - The right pane's picture space was the pixel wider an odd width leaves that pane. One picture on both sides, which a page two identical documents share is, was then asked for at two sizes, and the cache keeps one composite per picture: each paint composed both, each landing throwing the other away, for as long as the entry was on screen. Both pictures are now given the same width, which also fits two pictures of one size to one size. Five pixel baselines move with it, the right hand picture one pixel to the left.
Each handler wrote its event into one field of the input the next poll reads, and the pump dispatches everything AppKit has queued before it returns. So whenever a frame was slow, the loop waiting behind an accept on InlineApplier's mutex, two presses of Down scrolled once, Tab then a accepted the entry the reader meant to skip, and d then a click on another row discarded the clicked row, which the reader had never looked at: the managed side applies a frame's click before its key. Keys, clicks and menu events now go into a queue in Runtime, and deview_poll_input hands over the one that is next, as the WinForms head does. What adds up or says where it is now, the wheel, a drag, the scroller and a close, is still whole per poll. The ABI is as it was. Two things follow from handing over one a poll. The pump does not wait out its frame while more are queued, or a fast key repeat would be handed over more slowly than it arrives. And a context menu is not popped until the queue is empty: popping holds the managed loop for as long as the menu is up, so a key pressed straight after the right-click would be applied when the menu closed, however much later that was. Not compiled or run here. There is no Mac on this machine, so the macos-14 job is the first to build it, and nothing in a capture exercises input.
…ff them The footer was one row whatever was in it. The buttons were laid out left to right with nothing to stop them at the window's edge, and the status was drawn from the right edge whatever was already there. At the 1100 points the window opens at, an image pair in a queue had "images differ" drawn over its last button, and a paged document's eleven buttons needed 1405, so Zoom out and Zoom in were off the window and could not be clicked. A button that would pass the edge now starts another row, and the body gives up the height. The status is drawn beside the last row when there is room right of the last button, as before, and otherwise on a line of its own above the buttons, losing its end if it is wider than the window. Above rather than below, and with the buttons placed from their labels and the width alone, so they stay where they are as the status comes and goes: a click is resolved by position, and the status changes while the pointer is on its way. A footer whose buttons fit one row with room for the status beside them is laid out exactly as it was. That is every scene PixelTests captures, so no macOS baseline is expected to move. The managed side still slices the body as if the footer were one row. This head's own chrome leaves it 64 points to spare, which is three rows of buttons, or two and a status line. A paged document's buttons pass that in a window under about 600 points wide, or about 740 with a status too long for the last row, and the last one or two rows of the body are then not drawn. Not compiled or run here. There is no Mac on this machine, so the macos-14 job is the first to build it.
NSScroller tracks a press on its knob in a loop of its own, inside mouseDown, until the button comes up. That is inside the pump, so deview_present did not return for the length of the drag: the knob moved, every move was reported, and the managed loop that scrolls the panes in answer ran once, at the release. The scroller is now a subclass that takes a press on the knob as three ordinary events, the way the view takes a selection, so there is a frame between one move and the next and the panes scroll as the knob is dragged. It reports where the drag has got to and leaves the knob to the frame that answers, which puts it on the row the panes show. A press in the slot is still AppKit's: what it does is the reader's setting, and it is over in a click. When the knob's own rectangle cannot be read, the press goes to AppKit as before. A window being resized is the same kind of loop, and that one cannot be stepped around. The scroller is kept against the right edge and stretched by its autoresizing mask while it runs, where it used to stay wherever the last frame had put it. The panes still keep the rows they were sliced for until the mouse comes up, drawn into the new size. Doing better takes a way to ask the managed side for a frame from inside the loop, which is a change to the ABI, and the readme now says so. Not compiled or run here. There is no Mac on this machine, so the macos-14 job is the first to build it, and a capture makes no window, so nothing there exercises a scroller.
Core Text applies a font's calt feature unless told not to, and JetBrains Mono does its code ligatures through calt. So macOS drew <>, !=, <=, =>, -> and == as one glyph each, where the Windows and Linux heads draw a glyph a character: the title of a file comparison read <> there and as one diamond here. In a tool whose job is to show which characters a snapshot holds, that is the wrong answer, and most of all for the snapshot's own text. The font is now a copy with calt and liga off, through kCTFontFeatureSettingsAttribute. This moves the macOS pixel baselines whose text holds a sequence calt substitutes, and those have to be accepted again from the macos-14 job, since they cannot be made here. Worked out from the embedded font's own GSUB table, they are the five file mode scenes, for the <> in their title: FileDiff, Images, ImagesEnlarged, Selection and Minimal, which also has ... at each end of its folded rows. The five inline scenes hold no such sequence and should come out as they were. Not compiled or run here. There is no Mac on this machine, so the macos-14 job is the first to build it.
Following a drag of the knob only does anything if the scroller has a knob. A scroll view enables and disables its own scrollers, and this one is placed by hand with no scroll view around it, so nothing here ever said it was enabled: it was whatever an NSScroller starts as, and one that is not enabled draws its slot with nothing in it. Which that is could not be checked here. If a scroller starts enabled, this changes nothing. If it does not, the pane scroller has been an empty strip since it was added, and this is what gives it a knob. A commit of its own, so it can be dropped alone. Not compiled or run: there is no Mac here, and a capture makes no window.
…ame sweep just wrote A delete for a verified file stayed tracked when a later run failed against that file and a move onto it arrived. Accept all carries out the moves and then the deletes, so the received file was moved into place and then deleted: both gone, with nothing said. DiffRunner.SettleDelete withdraws such a delete, but not from a library that predates it, not while a viewer holds the queue the settle is sent to, and not while that port is remembered as unowned. A move now withdraws the delete it finds waiting for its target, whichever port it arrives on, since a run that verified against a file is what makes its delete stale. That leaves the delete that arrived after the move, where which of the two is stale cannot be known. Both sweeps, the menu's and the one a displaying viewer asks for, now leave a delete pending when a move in the same sweep wrote its file or a move still pending is going to. It is decided by what was written rather than by what was swept, so a move whose received file has gone, which is dropped without writing anything, holds nothing back.
…hose An accept-all takes its snapshots when it starts and read the deletes when their turn came, after the snapshots had been applied. A snapshot moving inline arrives as a patch plus a delete of the verified file the patch replaces, so a pair landing while a batch was applying had its delete swept by a batch its patch was never in: the verified file went while the patch was only pending. Both sweeps now list the deletes before anything is accepted, as the viewer's own batch does. The menu's lists them at the click, and the one a displaying viewer asks for lists them ahead of its snapshots and hands the keys to ITrackedFiles.AcceptAll. A delete that arrived during the batch stays pending and is counted as neither accepted nor kept. One that was listed and has gone since is passed over. Ahead of the snapshots rather than beside them, because Verify raises the delete and then queues the patch. What is left is a batch that begins between those two sends.
…answer Accept all asked the queue what was pending and read no answer as nothing pending: RemoteInlineHost.List flattens an owner that could not be asked to an empty list, which is right for a menu. So with a viewer holding the port and a patch, cold starting or wedged, and the tray holding the paired delete, the snapshot sweep was skipped and the delete was carried out. The verified file went with the patch that replaces it never tried. IInlineHost.TryList tells the three answers apart. Nothing holding the port is still nothing pending. An owner that is there and did not say what it holds is treated as a snapshot that was not written: the deletes are held and the user is told the viewer did not answer. A program that is not a viewer holding the port is no owner either, as it is for every send DiffEngine makes, so a tray that could not bind 3493 over one still carries out its deletes. ViewerClient already records that when a reply is not this protocol, and FoundUnowned lets the host read it.
Program.Main was async Task. The entry point the runtime starts for one of those is a method the compiler writes, which carries no attribute, so the thread every tray window lives on was MTA. Clipboard.SetText throws ThreadStateException there, which the debug view's catch for a busy clipboard does not match, so every click on Copy threw and nothing was copied. Main is now a synchronous [STAThread] method that blocks on Inner, which is what the compiler written entry point did. Nothing in Inner awaits until Application.Run() has returned, and WinForms has taken its synchronization context back off the thread by then, so the awaits that follow continue on the pool instead of waiting on a loop that is no longer pumping.
A tray that owns the queue stages what is pending as it exits, but only from the unwind after Application.Run() returns. A logoff or a shutdown never gets there. Windows sends each top level window WM_QUERYENDSESSION and then WM_ENDSESSION and may end the process once they are answered. The tray has no Form to turn those into a close, only the notify icon's window, so the loop kept running until the process was ended and the queue went with it. For a tray started at login that is how it usually stops. SessionEndWindow is a hidden top level window that hears WM_ENDSESSION and calls OwnedInlineHost.SessionEnding inside the message, which stages the queue before the message is answered. Patches arriving after that are refused, as an owning viewer refuses them once it is closing, so the sender stages what a process on its way out would otherwise have acknowledged and lost. Nothing is done for the query, or for an end that was called off, since a session that carries on would then have its queue both staged and held. Confirmed outside the repo with a tray shaped process: a notify icon window and no Form answers the query and Application.Run() has not returned two seconds after WM_ENDSESSION, while a hidden window made this way is sent both messages on the UI thread. A real logoff was not run.
A move for a pair the tray already tracks was always rebuilt from what the message carried. A Move or a Diff over the viewer port carries the two paths and nothing about the tool, so the tray filled the gap as it does for a pair seen for the first time, with its own choice for the extension, over a move that had already said which tool was showing the pair. "Open diff tool" on a viewer pair does this by hand: it starts DiffEngineViewer --diff, which cannot bind the port and forwards the pair to the tray as a Diff. The pair went from the viewer, not killable and open, to whatever the tray would pick, killable and with no window, so the "Accept open" hot key passed over a pair that was on screen in the viewer. A move that names no tool now changes nothing about the one recorded: the exe, its arguments, whether it may be killed, whether it kills what locks it and whether it is the viewer all stay, and only the target is taken. The focus race test told a focus from a Diff by the exe surviving, which a Diff now leaves alone as well, so it asserts on the entry not having been replaced.
… GetAwaiter An append always went on the end of the chain in C#, so await Verify(value).ConfigureAwait(false) was given a Snapshot call on a ConfiguredTaskAwaitable, which has none (CS1061), at a call site CanAnchor had already said could host a snapshot. Only F# stopped short of the end, and only at ToTask. The three members that hand back something other than a SettingsTask now end a chain in both languages, and the call goes in front of the first of them.
An append took the first entry point the search yielded. Once an accept higher in the file leaves the hint stale, the walk starts over from the member's declaration, and the first call it meets is the one most likely to have been accepted already: the patch was refused with 'already has a Snapshot call' while the call it was for sat two lines below, and a single accept dropped the entry. A call that has one is now passed over, along with any entry point inside its arguments. The call on the recorded line still decides, since a hint that lands on a call names it, and so does the nearest call when the patch names no member to bound the walk.
The appended call and a literal on its own line were both indented one level in from the line's leading whitespace. F#'s offside rule needs them to clear the column the expression starts at, and after do!, let! x =, let x = or a binding written on one line that column is further along the line: the appended call was read as the start of the next statement and the literal as offside, FS0010 either way, in source that compiled until it was accepted into. A chain ending on a closing paren or an argument at that column did the same to a call that does start its line. Both are now measured from where the expression starts, found by walking back over the call's receivers, and FsCompilerRoundTripTests compiles the shapes with dotnet fsi.
…variable
A Remove took the call off the end of whatever it hung off and kept the receiver. That is right for a verify call and wrong for a variable: settings.Snapshot("old"); became settings; (CS0201), and VerifySettings.Snapshot is public API that Verify sends a Remove for under NotInline. Where the call hangs off a name and nothing is chained after it, the statement's lines go instead, but only where that cannot change what is around them: alone on its lines, in a block and ending in its own semicolon in C#, and followed by another statement of the same block in F#. Anything else is reported rather than left as a bare receiver.
…ne names A queue entry is keyed by its line, and accepting a snapshot moves every later call site in the file. The re-run's patches then matched no key and were queued beside the entries they should have updated, so a bulk accept applied the stale content and refused the fresh, and a passing call that had moved onto a later test's old line settled that test's entry by key. An enqueue whose key names nothing now folds into the one entry for the same member, test, mode and anchor that its framework already has content in, and takes it to the new line. A key that names another member's entry is no longer believed: a settle leaves it unless the value settles it, staged trios are cleared by the same rule, and an enqueue updates its own entry instead. The viewer keeps the reader on an entry that moved. Two existing tests gave both call sites of one member the same literal, which now reads as one call site, so they have a literal each.
…ed to it Clearing staged trios could be scoped to a framework, but never was in practice. A test run stages through InlinePatchFile.Write with a patch that names no framework, and an unlabeled trio is cleared whichever framework asks. And the caller, Verify, could not pass an origin to InlineStaging.Clear, because the moniker the labels are written with is internal. So in a multi-targeted project with no viewer, the framework that passed deleted the staged snapshot of the one still failing. Write now stamps the framework of the process staging the file where the patch carries none, as the send to a queue owner does, and InlineStaging.Settle clears for the running framework alone. Clear is unchanged, which is what a retire wants.
…urned or passed
Removing a Snapshot call from a variable now takes the whole statement, because
settings.Snapshot("old"); otherwise became settings; which is no statement. But
every call on a variable with nothing chained after it was treated that way,
and one that could not be taken with its statement was refused. So
await task.Snapshot("old"); and var kept = task.Snapshot("old"); were refused
too, where taking the call alone leaves await task; and var kept = task; which
is what each would have run without the snapshot, and what it did before.
The statement goes only where the variable would be left as the statement.
Awaited, returned, assigned, compared or passed, the call alone is taken. A
lambda's body is still refused, since _ => _ is no body for a lambda that
returns nothing. In F# what takes the value has to sit on the same line: the =
a line above is the one a whole body hangs off.
…etter typed The Linux head read letters as the characters typed, 'a' for accept and 'A' for accept all. Caps Lock types 'A' with no Shift held, so with it on the key that accepts one snapshot accepted every pending one, with no confirmation, and d, v, q, n, p, m, r and j did nothing. The letter is now folded to lower case and accept-all is chosen by whether Shift is held down, which is how the Windows and macOS heads decide it. Shift with any other letter is that letter's command, as it is on those two.
…ws only filler A drag starts only where there is text to select, and that was asked of the left pane whichever pane was pressed: where its text starts is read from the first row that draws any, and a pane of nothing but filler has none. So nothing in the right pane could be selected or copied for a pending delete, whose left side is empty, or anywhere inside a long removed block. It is now asked of the pane the press landed in. A press in a pane of filler still starts nothing.
…do not fit Every footer button was drawn on one line and the status after the last of them, wherever that was. A paged document pending in a queue has eleven buttons, 1199 pixels of them in a window with 1084, so Zoom in could not be clicked and the status line was not drawn at all; in a window of its own the same document left the status 54 pixels. The status line is where the page on screen, a page that could not be drawn, a selection and a failed accept are said. Buttons now wrap onto another row at the window's edge, and a status with no room beside the last row takes a line of its own, cut with an ellipsis if even that is too narrow. The title stops short of the subtitle in the same way rather than running on under it. A footer that fits is laid out exactly as before, so the existing baselines are unchanged. Two scenes pin it, a document in a window of its own and the same one in a queue. Both are skipped on macOS, where they have no baseline.
…Linux The embedded font was the only font the Linux head had, so CJK, Hangul, Arabic, Hebrew, Thai and emoji all drew as the replacement glyph and a snapshot holding them could not be reviewed: a line with one such character changed looked the same on both sides. The other two heads fall back to system fonts. The window's font now takes the machine's fonts as further sources, found through fontconfig in the order it falls back through them. Only the fonts a character on screen has needed are read, on a thread of their own, and fontconfig is loaded at run time rather than linked, so a machine without it loads the library as before. Each is scaled to the embedded font's em, and a glyph wider than the cells the grid gave it is cut off where the next character starts. ImGui is built with 32 bit characters so that anything past the basic plane can be drawn at all. A capture never draws with them. It uses the embedded font alone, so a baseline is the same picture whatever a runner has installed, and a new scene pins that after showing the same text in the window. It is skipped on macOS, where it has no baseline.
A launched viewer took its working directory from the test host, which is usually the test project's output folder. On Windows a directory a process is in cannot be deleted, and a viewer hidden behind a tray lives for the session, so git clean -xdf or removing a worktree failed with nothing on screen to say what was holding it. The viewer now starts in the folder it runs from, which it holds anyway. A relative path on a --delete or --diff launch meant relative to the host, so it is rooted first; a rooted one goes over as the caller spelt it, since the row is settled by a key built from that spelling. DiffRunner.LaunchProcess is left alone: a third party tool resolves relative arguments against the directory it inherits, and its window is on screen for as long as it holds one.
ViewerLaunchGate waited for the port and then reported the launch whatever had happened to the process, so a viewer that could not take the launch still read as Launched. AddInlineAsync turned that into Queued, and a caller told Queued stages nothing: the snapshot was in no queue and in no file. A copy from before 20.5.0 exits with 2 on --payload, and an apphost with no runtime exits before any viewer code runs. Each held the gate for the whole five second wait and left its payload file in the temp directory. The launcher now hands back the process, and the wait ends as Failed as soon as that process has exited with a failure. A clean exit is still waited on, since a viewer that hands its work to an owner exits with zero, and one that is running and slow is still reported as launched. A failed inline launch takes its payload file back. Two things about which copy is started, from the same item. A wildcard over folders named for versions is taken highest first rather than most recently written, so the NuGet cache no longer yields whichever DiffEngine was restored last. And a copy stamped older than 20.5.0 is passed over while a newer one is further down the search order; it is still taken when it is the only one, and one an environment variable names is never second guessed.
…m the test host DiffRunner.LaunchProcess started a tool declared UseShellExecute: false through Process.Start, which on Windows always hands the child every inheritable handle whatever is redirected. One of those is the pipe dotnet test reads the host's output from, and it reads until every writer has closed it. So a run that opened the Word or Excel comparer, Cursor or VS Code's launcher did not return until the process it had started was gone, and the Word comparer's stays until Word is closed. This is Verify issue 1229 again, for the tools ShellExecute was not the answer for. On Windows those tools are now started by CreateProcess directly, with handle inheritance off and no console. ShellExecute with a hidden window was the other way to do it and is not used: the hidden request reaches the program as how to show its first window, so a console program that opens a window of its own never appears, and nothing in the file says which kind of console program it is. No console whatever the tool declared, because a console program sharing the host's console is given the host's standard handles with it, inherited or not. Tools declared with ShellExecute are untouched, and so is everything off Windows, where nothing in a definition says which tools need the terminal's streams.
The twenty seven bugs the six per-area reviews found are fixed in the commits before this one. todo.md loses them, and gains what each fix says it did not reach: the macOS changes have not been run, Verify has a call to change before staged trios are cleared per framework, and so on by area. - docs: which copy of the viewer runs now passes over one from before 20.5.0 when a newer one is there; the tray's Accept all says which deletes it carries out and which it holds; the inline page covers the F# indentation, what ends a chain in either language, a Remove that takes its statement, InlineStaging.Settle, and a settle or a re-run asking whose entry a line names. - claude.md says the same for whoever works on this next, and counts the shim's exports correctly: eleven, not eight. - OsSettingsResolver.Resolve documented one parameter and not the others, which a Release build makes an error. The Debug builds it was written under do not. - Three comments that the fixes made untrue: ILoopHooks on the native heads having no modal loops, the tray's DiffToolLauncher on what DiffRunner starts a tool with, and the viewer definition calling itself a console program.
SimonCropp
changed the title
Four ImageCacheTests failed together on CI's windows job, each having waited ten seconds for a decode to be posted back. Nothing in that run touched the WinForms head or its tests. A window's decode runs on the pool. So does every test, in parallel, and nine of these blocked where they stood until the decode posted back: each held a pool thread while waiting for work that needed one. Once every thread the pool had was held that way the work had nowhere to run until the pool grew another, which it does slowly, and on a runner of four cores busy with the other test projects ten seconds went by. The run before it passed with the same project taking 35 seconds, against under ten here, so it was already near. The tests now take what was posted from a channel and await it, which gives the thread back. The limit is a minute, since all it bounds is a test that would otherwise never end. The project runs in 5.5 seconds here where it took 9.5, which is the threads these were holding.
Clear runs once per passing inline verification, and what it does first is find every VerifyInline directory under the source project's obj. That is a walk of the whole tree, one directory listing per directory in it, to learn what it nearly always learns: that nothing is staged. The benchmark calls Clear the way Verify does, naming its intermediate directory, against a project in the temp folder whose obj has the shape of Verify.Tests' own: two configurations of five frameworks, 151 directories, ten of them an empty VerifyInline. Once with nothing staged anywhere, and once with five snapshots staged for other call sites. Before, on this machine, with other builds running beside it: | Method | Mean | Error | StdDev | Allocated | |-------------- |---------:|---------:|---------:|----------:| | NothingStaged | 12.69 ms | 2.385 ms | 0.131 ms | 124.17 KB | | OthersStaged | 11.90 ms | 8.477 ms | 0.465 ms | 129.74 KB | Where the project is decides most of that. The walk alone, timed outside the benchmark over a copy of Verify.Tests' obj, is 3.0 ms on the drive the repositories are on and 8.7 ms in the temp folder, which is on the system drive. So a thousand passing inline verifications spend between three and thirteen seconds finding nothing. dotnet run -c Release --project src/DiffEngine.Benchmarks -- --filter "*InlineStaging*"
…ng obj for each InlineStaging.Clear runs once per passing inline verification and began by listing every directory under the project's obj to find the VerifyInline ones. The answer was thrown away and worked out again for the next verification, a few milliseconds later. The list is now kept per project, on the two conditions that are why it was not kept before. Nothing this process has staged since. A run that finds no queue owner stages as it goes, and the call site it stages is one it will be asked to clear, so a kept "nothing here" must not outlive a write. Everything that writes a trio, InlinePatchFile.Write and Persist, counts itself, and a list taken before the last of them is not answered from. The count is read before the walk, so a write that lands during one is still seen by the next clear. And for a second at most, because another process can stage under the same obj and nothing tells this one: another framework of the same run, or a queue owner writing its queue out as it exits. A directory it creates is found by the first clear after that second. Only the list is kept, so a staging directory that already existed is read on every clear as it was, by its write time, and only one that did not exist can be late. The directory the caller names is still asked for every time. After, same machine, same conditions: | Method | Mean | Error | StdDev | Gen0 | Allocated | |-------------- |---------:|---------:|---------:|-------:|----------:| | NothingStaged | 418.7 us | 707.0 us | 38.75 us | - | 3.5 KB | | OthersStaged | 392.1 us | 669.9 us | 36.72 us | 0.4883 | 8.76 KB | From 12.69 ms and 124 KB. What is left is one write time per staging directory, eleven of them in this tree, which is the check that keeps a known directory's contents current. Two tests beside the one that already pinned the Persist half: what this process stages through InlinePatchFile.Write is found by its next clear, and what is written with nothing here involved is found once the kept list has had its life. Each of the three fails with its condition taken out, checked in a copy of the tree.
With no tray and no viewer open, which is the ordinary state of a machine between failures, everything ViewerClient does ends in a connect to a port nobody is listening on. Windows does not refuse that at once: the connect runs for two seconds, or for as long as the caller is prepared to wait. Five ways of meeting it: the launch gate's probe, the first telling send of a process in its sync and async forms, a whole gated call with nothing to launch, and a host asking for the queue. The memory of an unowned port is forgotten before each first send, since that memory is what stands between the first and every later one. The port is one the benchmark binds and never listens on. Nothing answers on it, a connect to it is refused exactly as one to a free port is (2,023 ms against 2,040 ms here), and nothing else can bind it while the run lasts. Never 3493. Before, on this machine, with other builds running beside it: | Method | Mean | Error | StdDev | Allocated | |----------------------------- |-----------:|----------:|---------:|----------:| | Probe | 512.1 ms | 234.38 ms | 12.85 ms | 1.53 KB | | FirstTellingSend | 2,030.8 ms | 119.79 ms | 6.57 ms | 4.29 KB | | FirstTellingSendAsync | 2,031.1 ms | 9.02 ms | 0.49 ms | 6.52 KB | | GatedCallWithNothingToLaunch | 506.0 ms | 46.67 ms | 2.56 ms | 4.26 KB | | Ask | 506.9 ms | 79.65 ms | 4.37 ms | 1.48 KB | There is no method for a send with an owner present, and that is on purpose. Each one is a real connection that leaves its port in TIME_WAIT, the dynamic range here is 16,384 ports, and a benchmark at a third of a millisecond a send makes more connections than that. Timed by hand over three hundred sends, it is 0.28 ms and 6.4 KB each. dotnet run -c Release --project src/DiffEngine.Benchmarks -- --filter "*UnownedPort*"
…hold A loopback connect to a port with no listener is not refused at once on Windows: it runs for two seconds, or for as long as the caller waits. The memory of an unowned port spared a test process every telling send after its first, and nothing spared it the first, or the launch gate's probe, or a host asking for the queue. So the first settle of every test process blocked for two seconds, and a gated call with nothing to launch held the gate for half a second, as did each poll while a viewer it had started was still binding. ViewerClient now asks the operating system first. With no listener on the port there is nobody to connect to, and it says so without trying. A listener there, or a table that cannot be read, leaves the connect to answer as before, so the table is only ever a way of not waiting. PiperClient already asked the same question of the tray's port, and the two now share ListenerTable. Three things about how it is asked. By port alone. A listener on an address a loopback connect does not reach costs the connect that would have been made anyway, where a rule about addresses that was wrong would cost a listener that was there. Only on Windows. Elsewhere the refusal is immediate, so the connect is the cheaper question and the one that cannot be wrong. Not for a port that accepted a connection in the last second. Reading the table means reading every connection the machine has, and each settle leaves one behind in TIME_WAIT for two minutes: timed apart from the benchmark, a read is 0.45 ms with 86 connections, 1.6 ms with 590, 4.0 ms with 1,595 and 7.9 ms with 3,102. Asked before every send, a green run with a tray answering would have paid more for each settle than for the one before it. As it is the first send to a port asks, and each connection that succeeds renews the second. A listener that binds just after the table was read is missed, as it was by a connect a moment early, and found the same way: nothing that asks is answered from the memory, so the gate's next poll reads the table again. The probe still records what it found, as it did. After, same machine: | Method | Mean | Error | StdDev | Gen0 | Allocated | |----------------------------- |---------:|-----------:|---------:|-------:|----------:| | Probe | 829.2 us | 1,165.2 us | 63.87 us | 1.9531 | 46.95 KB | | FirstTellingSend | 778.1 us | 188.6 us | 10.34 us | 1.9531 | 46.02 KB | | FirstTellingSendAsync | 648.1 us | 354.1 us | 19.41 us | 1.9531 | 45.97 KB | | GatedCallWithNothingToLaunch | 750.6 us | 813.0 us | 44.56 us | 1.9531 | 45.73 KB | | Ask | 723.2 us | 147.1 us | 8.06 us | 1.9531 | 45.84 KB | From 506 to 2,031 ms. Each is one read of the table and little else, so it moves with the machine: there were two to four hundred connections open during this run, and an earlier one with a thousand gave 2 ms and 190 KB. With an owner present nothing changed: 0.31 ms a send before and after, timed side by side over three hundred sends each, and 48 bytes more. Tests: a telling send to a free port returns in under a second, sync and async; an unreadable table leaves the connect to answer, for an owner and for nobody; a port that just answered is not looked up again, and one that has been quiet is. AConnectGivenUpOnIsObserved holds the table off its port, so that there is still a connect to give up on. Each new test fails with the piece it is about taken out, checked in a copy of the tree.
An accept-all applies one entry at a time, and each apply reads, lexes
and rewrites the whole source file. The review that raised this timed a
simulation of the applier's IO and allocations. This is the applier
itself, on a real file in the temp folder: InlineApplier.Apply for each
of 25 call sites in a 500 line file, and each of 500 in a 10,000 line,
600 KB file, every accepted literal moving the call sites below it.
Beside it, CanAnchor for each call site, which is what a test run asks
of a call site it has not seen before, and two that take the applier
apart: the patcher alone over the same patches in memory, and the lexing
alone.
Before, on this machine, with other builds running beside it:
| Method | CallSites | Mean | Error | StdDev | Gen0 | Gen1 | Gen2 | Allocated |
|-------------- |---------- |-----------------:|------------------:|---------------:|------------:|------------:|------------:|--------------:|
| AcceptEach | 25 | 259,350.40 us | 185,484.532 us | 10,167.033 us | 500.0000 | - | - | 9966.09 KB |
| AnchorEach | 25 | 4,969.46 us | 2,341.940 us | 128.370 us | 351.5625 | 109.3750 | - | 5812.61 KB |
| PatchInMemory | 25 | 2,458.12 us | 1,163.392 us | 63.769 us | 414.0625 | 167.9688 | - | 6781.37 KB |
| Lex | 25 | 54.05 us | 3.238 us | 0.177 us | 6.6528 | 1.6479 | - | 109.64 KB |
| AcceptEach | 500 | 28,423,459.23 us | 11,838,178.946 us | 648,890.537 us | 657000.0000 | 631000.0000 | 619000.0000 | 3863795.08 KB |
| AnchorEach | 500 | 1,135,102.43 us | 413,171.873 us | 22,647.345 us | 531000.0000 | 506000.0000 | 494000.0000 | 2236833.75 KB |
| PatchInMemory | 500 | 1,144,355.33 us | 131,406.115 us | 7,202.813 us | 534000.0000 | 507000.0000 | 496000.0000 | 2619652.66 KB |
| Lex | 500 | 1,092.22 us | 512.714 us | 28.104 us | 437.5000 | 410.1563 | 398.4375 | 2157.77 KB |
Twenty eight seconds for the five hundred, not the 3.8 the simulation
gave, and the patcher is 1.1 of them. The rest is the file system, and
nearly all of that is one call. Timed a step at a time outside the
benchmark, on files of the same two sizes in the same folder:
30 KB 600 KB
read 0.2 ms 0.9 ms
write temporary 0.2 ms 0.4 ms
File.Replace 9.5 ms 47.7 ms
everything else 0.2 ms 0.5 ms
The swap is not slow in itself. What it opens is a file written a moment
ago, and on a drive where new files are scanned that open waits for the
scan. A move that overwrites takes 0.4 ms instead, and the 38 ms then
turns up in the next apply's read.
The drive the repositories are on is not scanned, and there the same
Replace is 1 ms at either size. With TMP pointed at it this benchmark's
AcceptEach is 47 ms for the 25 and 3.2 s for the 500, which is the
simulation's number: 6.5 ms an apply, 2.3 of it the patcher.
So what an accept costs is decided first by how many times the file is
written, and the patcher's share shows once that is one. Within the
patcher, the 2.3 ms a patch of the large file takes goes on the passes
that read all of it: 1.2 lexing, 0.35 splicing, 0.25 and 0.23 finding
the line ending and the line starts, 0.14 the indent unit.
dotnet run -c Release --project src/DiffEngine.Benchmarks -- --filter "*InlineAccept*"
InlineApplier.Apply reads, lexes and rewrites the whole source file for each patch, and an accept-all calls it once per entry. The write is where the time goes: what the swap opens is a file written a moment ago, and where the drive is scanned that open waits for the scan. Five hundred snapshots in one 600 KB file took 28 s, 1.1 of them patching. InlineApplier.ApplyAll takes the patches together. Each file is read once, its patches are applied in memory in the order given, each to what the one before it left, and the file is written once through the same temporary and the same swap, with its lock and mutex held from the read to the write. The patcher is the one a single apply runs and it runs once per patch, so every patch is told what Apply would have told it in turn, and the bytes left are the bytes applying them in turn leaves: a test holds the two side by side over a batch with a moved call site, a repeated patch and a second patch for a call site already taken. Encoding, byte order mark and line endings come from the one read and go back in the one write. One thing can only be different. A single write carries every edit, so when it fails none of them happened: each patch from the first edit on reports the failure, including one judged already applied or not found after that edit, since what it was judged against never reached the file. A patch judged before any edit keeps its answer. Nothing is written when nothing applied. Apply is now ApplyAll's one-patch case, through the same code. InlineQueue.AcceptAll has an overload that hands every un-conflicted patch to the applier in one call, which is the library's own bulk accept able to use it. The tray and the viewer do not yet: both apply an entry at a time from their own loops, by design, and are not touched here. After, same machine: | Method | CallSites | Mean | Error | StdDev | Gen0 | Gen1 | Gen2 | Allocated | |--------------- |---------- |-------------:|-------------:|-----------:|------------:|------------:|------------:|-----------:| | AcceptTogether | 25 | 25.135 ms | 4.912 ms | 0.2692 ms | 406.2500 | 156.2500 | - | 6.75 MB | | PatchInMemory | 25 | 4.024 ms | 19.406 ms | 1.0637 ms | 414.0625 | 164.0625 | - | 6.62 MB | | AcceptTogether | 500 | 1,268.122 ms | 1,038.879 ms | 56.9444 ms | 533000.0000 | 514000.0000 | 495000.0000 | 2560.98 MB | | PatchInMemory | 500 | 1,301.252 ms | 448.650 ms | 24.5920 ms | 526000.0000 | 504000.0000 | 488000.0000 | 2558.28 MB | AcceptTogether is new, and is AcceptEach's work handed over in one call: 259 ms to 25 ms for the 25, and 28.4 s to 1.27 s for the 500. It now costs what the patcher costs, which is the row under it. AcceptEach itself is as it was, a single apply being the same work as before.
With the file written once, what an accept costs is the patcher, and half of that is the scan it builds over the whole source for every patch: a bool for each character, set one character at a time, and three hash tables saying where each comment and literal starts, where it ends, and which are comments. For the 600 KB file that was 2.2 MB a patch, most of it in arrays only the oldest generation collects, and with the two copies a splice made it came to a collection of that generation per patch. The map now marks what is not code and is filled a span at a time, so the lexers no longer touch it for the characters between. It is rented from the shared pool and handed back when the patcher has finished with the scan, which is why a scan is disposed. The spans are three lists in the order the lexer met them, and a span is found from either end by searching them, after the map has been asked whether the offset can be one at all. Nearly every offset a search steps over is code, so the usual question is one array read where it was a hash lookup. Splice makes one copy rather than two where the framework has the overload for it. What the scan answers has not changed. SourceScanTests walk every offset of two arranged sources and of the patcher's own test files, asking each question at each place, and fail on an off by one that all but four of the patcher's tests pass. Beyond the suite, every answer of the old scan and the new at every offset was hashed and compared over the C# and F# under six source trees here: 2,683 files, 9.8 million offsets, 66,702 spans, identical. After, same machine: | Method | CallSites | Mean | Error | StdDev | Gen0 | Gen1 | Gen2 | Allocated | |--------------- |---------- |-----------------:|-----------------:|---------------:|------------:|------------:|------------:|--------------:| | AcceptEach | 25 | 248,914.57 us | 60,059.49 us | 3,292.063 us | - | - | - | 6100.98 KB | | AcceptTogether | 25 | 19,598.27 us | 6,454.21 us | 353.777 us | 156.2500 | 31.2500 | - | 3047.88 KB | | AnchorEach | 25 | 4,265.25 us | 1,859.43 us | 101.922 us | 210.9375 | 23.4375 | - | 3541.83 KB | | PatchInMemory | 25 | 1,806.09 us | 317.57 us | 17.407 us | 177.7344 | 68.3594 | - | 2915.22 KB | | Lex | 25 | 29.57 us | 11.89 us | 0.652 us | 1.0986 | - | - | 18.76 KB | | AcceptEach | 500 | 26,140,940.97 us | 11,884,397.30 us | 651,423.921 us | 338000.0000 | 320000.0000 | 312000.0000 | 2293670.98 KB | | AcceptTogether | 500 | 1,043,330.20 us | 3,125,066.09 us | 171,295.418 us | 144000.0000 | 128000.0000 | 118000.0000 | 1050861.41 KB | | AnchorEach | 500 | 955,236.77 us | 2,225,161.30 us | 121,968.600 us | 182000.0000 | 165000.0000 | 157000.0000 | 1303198 KB | | PatchInMemory | 500 | 792,547.53 us | 167,034.42 us | 9,155.720 us | 139000.0000 | 121000.0000 | 113000.0000 | 1048287.11 KB | | Lex | 500 | 613.34 us | 274.12 us | 15.026 us | 17.5781 | 4.8828 | - | 289.04 KB | Lexing the large file goes from 1,092 us and 2.2 MB to 613 us and 0.29 MB, and a patch of it from 5.2 MB to 2.1 MB with a quarter of the collections. The times of the methods that patch are down by between a sixth and a third, which is inside the error of a run on this machine today. The allocations are exact. On the drive the repositories are on, where no scan of the file hides it, with TMP pointed there: | Method | CallSites | before | after | |--------------- |---------- |------------:|-------------:| | AcceptEach | 25 | 46.77 ms | 38.15 ms | | AcceptEach | 500 | 3,226.27 ms | 1,880.06 ms | | AcceptTogether | 25 | | 4.00 ms | | AcceptTogether | 500 | | 739.81 ms | So an accept-all of the five hundred, which the tray and the viewer still make an entry at a time, is 3.2 s to 1.9 s for them as they are, and 0.74 s once they hand a file's entries over together. What is left of a patch is the passes that read all of its source: the lexing, the line starts, the line ending, the indent unit and the one copy. CanAnchor makes them too, after reading and decoding the file, once for each call site a run has not seen before.
InlineApplier.ApplyAll reads a source file once, applies its patches in
memory and writes it once, and nothing that runs an accept-all called it: the
viewer's batch and the tray's both applied an entry at a time, each a read, a
lex and a rewrite of the whole file. The rewrite is what costs. A file written
a moment ago is scanned by whatever watches the drive before the next thing
can open it, so five hundred snapshots in one file were half a minute of
writes around a second of patching.
Both now hand a file's snapshots over together.
The viewer's batch claims, with a snapshot, every other one it still has to do
in the same source file (AcceptBatch.Together), applies them through
ViewerActions.ApplyTogether, and records each outcome as it would have one at
a time. Claimed, rather than looked ahead to, so a settle, a discard or a
re-run that lands while the file is being written is noticed for each of them
as it is for a single entry. A group's batch takes only its own.
The tray's AcceptEvery looks up, when a snapshot's turn comes, the others
still to do in its file, and completes them together.
Each snapshot still has its own outcome, and the batch's rules decide what
becomes of it. Two things do change, and both are what "a file at a time"
means. Progress moves by a file's snapshots at once. And the moment up to
which a snapshot can still be withdrawn is its file's turn rather than its
own: one discarded, or settled by a test that started passing, while its own
file is being written was handed over with the rest and is written with them.
It is not counted, and a discard still takes it out of the queue. The tray's
test for an entry discarded part way now has the two in different files, and
a second test says what happens within one. Before, that moment was the
entry's own apply; the batch it is part of is now many times shorter.
A test's applier is still asked about every snapshot: with only ApplyInline
supplied, or the tray's applier seam, together means each in turn.
GroupAcceptBenchmarks, "Accept all for" a test in one source file, by the real
applier, until the last snapshot is written:
one transition a batch, an entry a batch, a file
at a time at a time
20 snapshots 51 ms 46 ms 2.8 ms
200 snapshots 1,252 ms 1,285 ms 26 ms
and 9.8 MB allocated for the 200 where the one transition allocated 29.5.
With other builds running.
…raw made A spinner turns by invalidating its own rectangle, some twenty times a second for as long as a page is being drawn. Renderer.draw took no notice of what it was being asked to repaint: every turn built the attributed string and the line for every piece of text in the window, about a hundred and fifty of them. And text outside Latin is a line a character, since the managed side sends every character CellGrid.Simple leaves out as a segment of its own. What AppKit hands the view for such a turn could not be found out here, so there are two changes, one for each answer. Where the clip is narrowed to what was invalidated, draw now leaves the rest out. It reads the bounds of the context's clip, and a row, a queue row, a line of text and a picture are skipped when their own rectangle does not touch it. It is the clip that is asked, at each draw, rather than what was invalidated or the rectangle the view is passed, so nothing that would have been painted can be left out: nothing outside the clip is painted, whatever is drawn. The test is of everything the caller would paint, so that something partly inside is still drawn. A row is asked about with its gutter, which a pane narrower than a gutter draws past the row's own edge, and a picture with its outline, which is the point outside it. Text is clipped to its own rectangle already, so that rectangle is the whole of it. The background, the rules, the buttons' faces and the spinner are drawn regardless and left to the clip, each being a call or two. The layout that comes back is still the whole window's, since the view resolves clicks, tooltips and cursors against it: an enlarged picture said where it was after drawing, and now says it before. A capture has no clip to read and is given none, so it leaves nothing out. Where the clip is not narrowed, the whole window is drawn, as it has to be, and that looks to be the usual case. Since macOS 11 a layer backed view whose backing store AppKit manages is reported to be handed its whole bounds whatever was invalidated, clip included, and Apple has called that the expected behaviour (developer.apple.com/forums/thread/663256). On macOS 14.0 to 14.3 any invalidation was the whole view, and later versions still ask for all of it now and then (developer.apple.com/forums/thread/738042). So the lines are kept as well: the ones the last two draws drew, found by their text and colour. A draw that draws the same text again makes none of them, which is a turn of a spinner, a dragged picture or splitter, and all but a row of a scroll. One found from the draw before is carried forward, and one that was not drawn again goes when the next draw begins, so what is kept is two draws' worth of lines however long the session: a window of text twice over at the most. A draw that reached no text is passed over, so that where the clip is a spinner, a turn leaves the lines as they were for whatever is drawn after it. The todo sketched keeping the lines of single clusters only. That was on the footing that a turn lays out nothing else once the clip is honoured, and it is every line instead because of the above. Single clusters are still the ones it saves most on: one line for every character of a pane of Chinese, most of them the same few hundred. A line is made from its text, its colour and the font. The font is the renderer's for its whole life, so the first two are the key. The text is compared by its bytes rather than as Swift compares strings, which holds a composed character equal to the same one decomposed. The colour is compared by which object it is, and the palette now hands out one object for each of the three colours a changed row's text is drawn in, where it made one on every call. Comparing colours through Core Foundation instead would rest on Core Graphics hashing equal colours alike, which could not be checked here, and a key whose hash and equality disagree is one a dictionary can trap on. A capture draws from the same table. A kept line is the line that would have been made, so nothing it draws moves, and the captures CI takes one after another now draw the labels they share from it. Widening CellGrid.Simple, the third fix the todo sketches, is managed code and is not here. Written without being compiled or run. There is no Mac on this machine, so the macos-14 job is the first to build it. That job only captures, and a capture has no clip and no window, so nothing there exercises the skipping at all.
… size An enlarged picture that is still below its own size was drawn straight from the decoded picture at .high, which resamples every source pixel under the clip on every redraw. A drag is a redraw a frame, so dragging a pair of 2880 pixel wide screenshots at 150% resampled both panes on each frame, to change nothing but where they are. Scrolling the rows above the pictures, or selecting in them, cost the same. The window now draws such a picture from a copy scaled to the device pixels the whole of it takes. The copy is made on the work queue by the code that makes the fitted copy, and kept where that one is. It turns on the picture's file and stamp and on that size, never on where the picture has been dragged to, so a drag draws the same copy somewhere else. The copy is put a pixel to a pixel: on the pixel nearest to where the drag left the picture, and at the copy's own size, which is the picture's to the nearest pixel too. Drawn into the exact rectangle it would be sampled again on every frame, each pane softening its own by a different fraction of a pixel, or, unsmoothed, losing or doubling a row or a column where the two sizes part. Every part of it stays within about a pixel of where it was. Nearest takes some care. Where a picture has been dragged to goes to the managed side and back as a Float, and where it should be drawn is often exactly half way between two pixels, so rounding it as it stands took a picture dragged a pixel at a time by none or by two. The position is taken to an eighth of a pixel first, and a half goes up whatever its sign. A model of the drag's arithmetic, in Python, had two drags in five wobble without that on a plain display and none with it. It is not a test of the Swift. Until the copy lands the picture itself is drawn at .low, and drawn again when it lands, as a fitted picture is stretched from its last copy while a window is resized. So a step of zoom is a quick rough frame or two and then the copy, where it was a slow frame every time. The copy is asked for before the picture is tested against the clip, as a fitted one's is, so a draw that leaves the picture out has still started it. What is kept is one copy a picture, in the slot the fitted copy had, so the two are never both held. Four bytes a pixel, at a size narrower than the picture's own, since a copy is only made below that: about what the decoded picture itself comes to, at the most. Past its own size the picture is drawn from as before, with .none. A 2880 by 1800 screenshot that fits its pane at 1040 pixels across is 6.1 MB at 150% (1560 by 975) and 10.8 MB at 200% (2080 by 1300), beside the 20.7 MB the decoded picture already is, and at 300% it is past its own size and there is no copy. A pair is twice that. What evicts it is a copy for another size landing: another step of zoom, a resized window or pane, a display of another scale, or going back to fitted, which makes the fitted copy again where it used to be still there. Or the picture leaving the frame, which drops its entry at the next draw, or its file being rewritten, which replaces the entry. Zooming past the picture's own size does not: the copy stays, unused, until one of those. Not when both panes name the one picture, which a byte equal pair of documents does, their pages being kept under the document's hash. With one copy a picture and panes that can be a point apart in width, each copy that landed would be the wrong size for the other pane, and the two would be made in turn without end. Such a pair is drawn from the picture, as before. By a reading of fitted, a fitted pair of them takes those turns already; that is left as it was. The copy is sRGB, as the fitted one is, where the picture itself is drawn in whatever space its file names. A capture is as it was. It draws from the picture, by the statement it always did, and asks for no copy, so no baseline moves. Written without being compiled or run. There is no Mac on this machine, so the macos-14 job is the first to build it. That job only captures, which this leaves alone, so nothing there runs the new path.
DiffEngineViewer.Benchmarks measures the viewer's model, and builds everywhere the model does. What a paint costs is the head's, and the WinForms head only builds for Windows, so its benchmarks cannot live there. This is the project the other two already name: both InternalsVisibleTo files list DiffEngineViewer.Windows.Benchmarks, and DiffEngine.Benchmarks' Program.cs says it exists. It is set up as the other two are: signed, in process and a short job. It targets net10.0-windows with WinForms as the head's tests do, references the head as they do, and is left out of the Release-NotWindows build with them. It needs no aliased reference to DiffEngine, which the tests have only to displace the copy Verify brings in. The head is an executable with a Program of its own, which this assembly can see, so the benchmarks are found from the executing assembly rather than from typeof(Program). No benchmarks yet: each arrives in a commit of its own, ahead of the fix it measures.
The first benchmark of the WinForms head, of the canvas as it is before the fix that follows. Nothing scrolls sideways, so all of a row past its pane's right edge is never seen, and a paint should cost what the part that shows costs. It costs by the length of the row instead, in two ways, and each has a shape here: LongLines is 72 rows of 2,000 characters. The canvas clips a row to the pane's width in pixels, counted as characters, so GDI+ is handed 472 characters of each row where 54 show, and 1,528 where 174 show at 1600 pixels a pane. It lays out every one before it clips any. MegabyteLine is a megabyte of one line, with one character from outside ASCII half way along it. A row is segmented before anything is clipped, which for a row that is not all ASCII walks every character of it, and the run before that character is then copied out whole to be cut down to what shows. OffscreenCanvas is what paints. It asks the canvas to paint into a bitmap through Control.InvokePaint, so what runs is the canvas's own OnPaint and nothing is put on the desktop: no form, and no window but the one WinForms parks a parentless control on when its font is measured. It is at one scale whatever display it runs on, as the tests' host is. Method Width Mean Allocated LongLines 1100 18.01 ms 77.21 KB MegabyteLine 1100 28.14 ms 2055.44 KB LongLines 3212 70.87 ms 226.12 KB MegabyteLine 3212 27.73 ms 2058.60 KB Timed on a machine busy with other builds, so the means are loose. A second run gave 18.72, 27.54, 83.11 and 29.56 ms, and the same allocations.
A row was clipped to its pane's width in pixels, counted as characters, so a long row went to GDI+ almost nine times the length of what shows of it, and GDI+ lays out every character it is handed before it clips any. And the row was segmented before it was clipped at all, which for a row that is not all ASCII walks every character of it and copies out the run it then cuts down. The canvas now asks for the start of the row: RowText.Shown flattens and cuts it where the pane's cells end, at a boundary CellGrid would cut at, reading from the front and only as far as that takes. A megabyte row costs what its first sixty characters cost, tab or no tab. One cell past the last that shows is kept. GDI+ fits glyphs to whole pixels, which can start one in the last pixel of the cell before its own, and with no cell to spare a W there lost two pixels at one pane width in ten. With it, the first cell left out starts a whole cell past the edge, and since where GDI+ puts a glyph does not depend on what follows it in the string, nothing drawn moves: the frames the benchmark paints are byte for byte what they were, and the pixel baselines pass untouched. ARowCutAtItsPaneIsDrawnAsTheWholeOfItIs holds it there, comparing a pane's rows with a canvas wide enough to draw them whole, across twenty widths a pixel apart. One thing is drawn that was not, which that test found. What is left of the old clip is a bound on a single character with marks without end, and it counted the pixels left of the pane from where the character starts. An emoji is two UTF-16 units, so one whose first column of pixels was the pane's last was cut to nothing, and a joined sequence within its own length of the edge lost the last of what it joins. The bound is the pane's whole width now. Method Width Mean Allocated was LongLines 1100 3.50 ms 28.27 KB 18.01 ms 77.21 KB MegabyteLine 1100 0.34 ms 1.43 KB 28.14 ms 2055.44 KB LongLines 3212 11.31 ms 62.04 KB 70.87 ms 226.12 KB MegabyteLine 3212 0.87 ms 2.37 KB 27.73 ms 2058.60 KB On a machine busy with other builds, as before. A second run gave 3.84, 0.34, 10.06 and 0.96 ms, and the same allocations. RowText.Shown is in the model because what it has to agree with is CellGrid, not anything of this head's. Its tests are with the head's all the same, since this head is the one caller: that the start is what cutting the whole row gives, for every kind of character the grid treats differently and every cut, and that it is found without reading the rest, counted in allocations rather than timed.
Of the canvas as it is before the fix that follows. A picture enlarged past its pane is drawn a part at a time, straight from the decoded picture, on every paint. Past its own size that is the pane's worth of pixels. Below it, the part that shows is still several times the pane's pixels, and the filter that makes a reduction look right reads every one of them: the paint costs by the picture, not by the pane, on every frame of a drag and on every wheel notch over the text of a document whose page is drawn under it. A pair of 4000 by 3000 pictures in the window a viewer opens at, where they fit at about an eighth of their own size. Percent is how far in, of the size that fits: 150 and 200 draw the picture at a fifth and a quarter of its size, 400 at a half. Still is a paint with nothing changed, and Dragged one where the picture has moved since the last, as each frame of a drag is. Method Percent Mean Allocated Still 150 53.57 ms 1.20 KB Dragged 150 52.76 ms 2.00 KB Still 200 37.10 ms 1.38 KB Dragged 200 39.78 ms 1.93 KB Still 400 21.36 ms 1.29 KB Dragged 400 22.30 ms 1.82 KB The quietest of three runs on a machine busy with other builds. The other two were between 52 and 79 ms at 150, 42 and 59 at 200, and 25 and 34 at 400.
…t every paint A picture zoomed into, but still drawn below its own size, was scaled from the decoded picture on every paint, with the filter a reduction needs, which reads every pixel it reduces. So a paint cost by the picture rather than by the pane: 54 ms for a pair of 4000 by 3000 pictures at 150%, on every frame of a drag. While it is drawn at half its own size or less, the whole picture is now scaled to that size once, on the pool as the fitted one is composed, and kept in its place. A paint copies the part that shows out of it. Where the picture has been dragged to is not part of what is kept, so a drag copies a different part of the same thing. It is at most a quarter of the picture's own pixels, replaced when another size is asked for, and gone with the picture when it leaves the screen or its file changes. Until one lands, the last thing composed is drawn stretched, as it is mid resize. Past half its size nothing is kept and it is drawn straight from the picture as before. The whole of it there would be up to the decoded picture's size again, which for a pair of 4000 by 3000 pictures is up to 96 MB more, and the part that shows is under four times the pane's pixels. The todo suggested plain bilinear for that range. Measured, it is the slower of the two in GDI+: 14 ms a pane against 9 for the filter already there, so that stays. Two things came with it. The checkerboard under an enlarged picture was a brush tiling one pair of squares, which was most of what was left: over 3 ms a pane, where filling the pane with one colour and half its squares with the other, as one list, is half a millisecond. It is the same pixels. The composed checkerboard goes through the same list, and a list of no squares, under a picture no larger than one, is not handed to GDI+, which throws on it. And a composite is now known by what built it as well as by its size, since a fitted one, with its checkerboard, at the size an enlarged one is asked for is still not that. The copy is placed on whole pixels. A picture centred in a pane an odd number of pixels wide starts exactly half way between two and stays there for the whole of a drag, where rounding to the nearest fell either way on the arithmetic's last digit: it stood still for one pixel of the drag and jumped two for the next. Half way goes up now, by a margin. Method Percent Mean was Still 150 1.99 ms 53.57 ms Dragged 150 1.97 ms 52.76 ms Still 200 1.98 ms 37.10 ms Dragged 200 2.08 ms 39.78 ms Still 400 17.04 ms 21.36 ms Dragged 400 17.34 ms 22.30 ms Allocations are as they were, under 2 KB a paint. A second run gave 1.92, 1.89, 1.98, 1.95, 15.18 and 15.33 ms. 400 is the range nothing is kept for, and what it gained is the checkerboard. No pixel baseline changed: every scene of WindowsPixelTests was captured and is pixel for pixel what is committed, the two enlarged ones included, which are past their own size and share only the checkerboard with this. Nothing there draws a picture below half its size, so the benchmark's frames were compared before and after instead. At 400 they are identical. At 200 the still frame differs by one level in 1,578 pixels. At 150 it differs by up to 24 levels on the lines of the grid, 0.9 on average, the copy being 608 high for a picture placed 607.5 high. The dragged frames, whose positions fall between pixels, differ by more on those lines, which are under a pixel wide: the copy is up to half a pixel from where the exact placement is. EnlargedPictureTests holds it through the real canvas: scaled once however it is dragged, nothing kept past half its size, every change of stripe drawn within a pixel and a half of where the placement puts it at each step in, a pixel moved for each pixel dragged, and in a window no paint scaling anything itself.
IdleFrameBenchmarks hands a form that is never shown the screen it was handed last, and a screen built again from the same state, which is what every idle frame handed over while the loop built one a frame: the screen of the frame before 0.7 ns an equal screen built again 370 ns The second is the field by field comparison that stood between an idle frame and a repaint. It was the small part of what such a frame cost, behind building the screen at all. The one ImageCacheTests test that arrived with the enlarged picture work waits for its decode as the others now do, without holding a pool thread.
…rmance changes In claude.md: the listener table in front of a connect, a file's snapshots written together and what that does to the moment one can be withdrawn, the scan's rented map, the staging directories kept a second, a batch's record step, the WinForms head's rows cut to their pane and its scaled copy of an enlarged picture, and the macOS head's clip test, kept lines and reduced copy, with what CI does and does not exercise of them. In the viewer's page: an accept-all goes a step at a time, a file's snapshots are one step, and a header's accept goes the same way.
The checkerboard behind a picture is drawn as a quad a dark square, built again every frame, for opaque pictures too. What that costs was an estimate, because nothing measured the shim: it only builds and runs on Linux, and the benchmarks ran on Windows. NativeFrameBenchmarks opens the head's real window through NativeViewerWindow and turns it as ViewerProgram's loop does, one Present and one Poll, on a comparison of text and on two comparisons of pictures that fill their panes. BenchmarkDotNet's in process runner gives each case a thread of its own for its setup, iterations and cleanup, so a window per case keeps every call into the shim, and into OpenGL, on the thread that owns the GL context. The time on the clock says little, since the shim holds the loop to sixty frames a second, so three columns are added. CPU is the processor time of the whole process around each turn, since llvmpipe rasterises on several threads. Triangles is OpenGL's own count of the primitives generated during a turn, which is what the shim submitted whatever then filled them, and Drawn is how many turns generated any. X server CPU is what the server spent over the same stretch. Mean is made to mean something by presenting each measured frame more than a sixtieth of a second after the one before, so it has nothing to wait out: an IterationSetup, which BenchmarkDotNet answers by running one frame an iteration. The two frames presented in turn differ by a letter of the status line, so a frame stays a frame the head has to draw once it learns to leave an unchanged one alone. Left out of a run wherever there is no shim to load or no display, which is every run on Windows, by a filter on the class. Before the fix, in an ubuntu:24.04 container set up as the unix job is, under Xvfb with llvmpipe on four threads, 30 iterations: LIBGL_ALWAYS_SOFTWARE=1 GALLIUM_DRIVER=llvmpipe LP_NUM_THREADS=4 \ xvfb-run -a --server-args="-screen 0 3840x2160x24" \ dotnet run -c Release --project src/DiffEngineViewer.Benchmarks -- \ --filter "*NativeFrame*" --warmupCount 5 --iterationCount 30 | Method | Window | Mean | StdDev | CPU | X server CPU | Drawn | Triangles | Allocated | |-------------------- |---------- |----------:|----------:|----------:|-------------:|------:|----------:|----------:| | Text | 1100x700 | 3.655 ms | 0.1268 ms | 9.51 ms | 0.37 ms | 1 | 5588 | 4.24 KB | | OpaquePictures | 1100x700 | 6.668 ms | 0.1939 ms | 19.4 ms | 0.45 ms | 1 | 9522 | 4.38 KB | | TranslucentPictures | 1100x700 | 6.833 ms | 0.3882 ms | 19.07 ms | 0.48 ms | 1 | 9570 | 4.38 KB | | Text | 3840x2160 | 23.273 ms | 1.9425 ms | 63.25 ms | 4.26 ms | 1 | 19492 | 18.88 KB | | OpaquePictures | 3840x2160 | 79.265 ms | 2.3843 ms | 201.92 ms | 4.51 ms | 1 | 113834 | 4.38 KB | | TranslucentPictures | 3840x2160 | 79.156 ms | 1.7718 ms | 199.45 ms | 4.69 ms | 1 | 113882 | 4.38 KB | todo.md put the checkerboard at about 4,400 quads for two panes at the size the window opens at and about 53,000 maximised at 4K, which is 8,800 and 106,000 triangles. A frame of two pictures submits 9,522 and 113,834, and an opaque pair costs what a translucent one does.
… shows The Linux head drew the checkerboard as a light rectangle and a quad for every dark square over it, built again each frame through ImGui's draw list and rlgl's vertex calls: 4,488 squares for two pictures at the size the window opens at and 56,644 for two filling the panes of a 4K window, sixty times a second, behind opaque pictures as well. It is now a two by two texture of the two tones, sampled as its texels and set to repeat, on one quad the size of the picture with texture coordinates that run to the picture's size over two squares. The squares still count from the picture's own top left corner, enlarged and panned or not, as the quads did. And it is drawn only behind a picture some of which can be seen through. That is asked of the decoded pixels rather than of the format, since a screenshot or a page of a document is usually saved with an alpha channel that is 255 throughout: one pass over them as the picture is decoded, on the decoder's thread for a window. Behind a picture with no such pixel every square was under an opaque one, and filling them cost a software rasteriser the picture's area a second time. The pixels are the same. The fifteen Linux baselines are the same files, byte for byte, with the shim built before and after. So are 179 further captures made of both outside the repository: pictures with clear alpha, opaque alpha, no alpha, grey with and without alpha and a JPEG, fitted at their own size and at a fraction of it, at three window sizes, and at every step of enlargement about several centres. And so are ten photographs of the X screen showing the window itself, which is given its pictures by the decoder's thread where a capture decodes its own. The 4K captures of the large pictures needed a line added to the old shim to be compared at all. A capture's ImGui context does not declare ImGuiBackendFlags_RendererHasVtxOffset, which the window's does, so the old checkerboard took a 4K capture's draw list past 65,535 vertices and it came out scrambled. Nothing captures at that size, and the checkerboard no longer gets a capture there, but dense text could, and the flag is still not declared. After, with the same command as the commit before: | Method | Window | Mean | StdDev | CPU | X server CPU | Drawn | Triangles | Allocated | |-------------------- |---------- |----------:|----------:|---------:|-------------:|------:|----------:|----------:| | Text | 1100x700 | 3.915 ms | 0.1840 ms | 10.4 ms | 0.45 ms | 1 | 5588 | 4.24 KB | | OpaquePictures | 1100x700 | 4.429 ms | 0.5065 ms | 13.97 ms | 0.48 ms | 1 | 542 | 4.38 KB | | TranslucentPictures | 1100x700 | 4.392 ms | 0.2574 ms | 13.86 ms | 0.42 ms | 1 | 594 | 4.38 KB | | Text | 3840x2160 | 24.615 ms | 0.8391 ms | 68.16 ms | 4.77 ms | 1 | 19492 | 18.88 KB | | OpaquePictures | 3840x2160 | 28.096 ms | 0.9691 ms | 95.51 ms | 4.06 ms | 1 | 542 | 4.38 KB | | TranslucentPictures | 3840x2160 | 34.061 ms | 0.3450 ms | 118.2 ms | 3.71 ms | 1 | 594 | 4.38 KB | A frame of two pictures went from 6.7 ms to 4.4 at the size the window opens at and from 79 ms to 28 at 4K where they are opaque and 34 where they are not, and from 9,522 and 113,834 triangles to 542. Text is the same frame as before, and what its numbers moved by is the noise of a machine that was doing other things.
A viewer left open on a comparison presents the same screen sixty times a second, and the shim builds, draws and puts on the screen every one of them. Under a software rasteriser or over a remote session each is the whole window filled and copied again, and what that adds up to had not been measured. NativeIdleBenchmarks turns the loop sixty times on an unchanged screen, with nothing arriving in between, and that second is one operation: CPU is what a second of an idle window costs the process and X server CPU what it costs the server, Drawn is how many of the sixty frames were put on the screen, and Mean is how long the sixty took, which is a second for a head keeping to sixty frames a second and more for one that cannot. The counting moves for it. Each turn now has an OpenGL query of its own, and none is read until the operation's turns are over and the clock has been read. Reading one has llvmpipe finish whatever it is holding on all its threads, and for a turn that draws nothing that is work the question made: a few tenths of a millisecond of processor time a turn, which nothing notices beside a frame that is drawn, and which would be most of what a turn costs once one is not. The processor time is shown to the thousandth for the same reason, since it is shown in Mean's unit and Mean here is a second. Before the fix, in the container and under the rasteriser the frame benchmark was run in, ten iterations: LIBGL_ALWAYS_SOFTWARE=1 GALLIUM_DRIVER=llvmpipe LP_NUM_THREADS=4 \ xvfb-run -a --server-args="-screen 0 3840x2160x24" \ dotnet run -c Release --project src/DiffEngineViewer.Benchmarks -- \ --filter "*NativeIdle*" --warmupCount 3 --iterationCount 10 | Method | Window | Mean | StdDev | CPU | X server CPU | Drawn | Triangles | Allocated | |--------- |---------- |--------:|---------:|--------:|-------------:|------:|----------:|----------:| | Text | 1100x700 | 1.004 s | 0.0009 s | 0.554 s | 0.02 s | 60 | 336480 | 254.53 KB | | Pictures | 1100x700 | 1.005 s | 0.0007 s | 0.868 s | 0.027 s | 60 | 36600 | 33.42 KB | | Text | 3840x2160 | 1.346 s | 0.0518 s | 3.69 s | 0.243 s | 60 | 1170840 | 903.42 KB | | Pictures | 3840x2160 | 2.133 s | 0.0633 s | 7.511 s | 0.244 s | 60 | 36600 | 33.42 KB | An idle window at the size it opens at takes more than half a core, and a maximised 4K one keeps all four of the rasteriser's threads busy and still cannot draw sixty frames in a second: the sixty take 1.3 s over text and 2.1 s over pictures.
…creen deview_present built, drew and put on the screen a frame sixty times a second whether or not anything had changed since the one before. Under a software rasteriser or over a remote session each of those is the whole window filled and copied again, and an idle window took more than half a core at the size it opens at and all four of the rasteriser's threads at 4K. A frame is now drawn only when it differs from the one on the screen, and built only when something could have changed it. Drawn or not is decided from what ImGui produced: every frame that is built is reduced to a fingerprint of its draw lists, and one whose fingerprint is the last drawn frame's is not put on the screen again. So a pointer crossing a pane, a key that did nothing and the frames that follow any change cost what building them costs. A frame is drawn regardless when ImGui is waiting for a texture, when a picture has just been uploaded, and when the window cannot be taken to be showing what was last drawn into it: nothing drawn yet, another size, shown after being hidden or minimised, or asked for by the window system. raylib sets no refresh callback, so GLFW's is set here, and a window another has been over, or that was unmapped, is drawn again. Built or not is the narrower test. No frame is built once the screen handed over is the last one byte for byte, nothing has come from the pointer, the keys or the window, nothing has landed from the decoder or the font finder, no tooltip is waiting out its delay, the files behind the pictures on the screen are as the last frame found them, and sixty frames built in a row since any of that last changed have each come out as the frame on the screen. That last is what a spinner fails, and anything else that moves by itself, and a second is longer than anything ImGui does over several frames or times by the clock it is given. A frame that is not drawn, or not built, still ends as a drawn one does: it waits out what is left of a sixtieth of a second and then reads the window system's events. EndDrawing did both, but only for a frame it had put on the screen, so it is no longer called and the end of every frame is Rest: the buffers are swapped where there is a frame to show, and the wait and the read follow in raylib's order. The managed loop still turns sixty times a second, the close request, the keys and the wheel are still read once a turn, and the window's placement is still noted every turn. ImGui is told the time since the present before, built or not, rather than raylib's frame time, so a window left alone for an hour does not find every delay served in its first frame back. Checked in the container beyond the suite. The fifteen Linux baselines are the same files byte for byte. The real viewer was driven under Xvfb with no window manager, by the pointer and the keys, with this shim and the one before it: three sessions, 46 photographs, every one the same picture with both. A tooltip comes up with the pointer at rest and stays up, a context menu opens, lights the row under the pointer and closes on Escape, the scrollbar drags, text stays selected, the wheel scrolls and enlarges, a snapshot arriving over the socket is shown, a picture's file written again, deleted and put back is followed, the part of the window another window was over is drawn again when it goes, as is a window unmapped and mapped and one resized, a window moved and then closed writes down where it was, and a close request ends the viewer within 200 ms. A spinner keeps turning, a window hidden and shown or focused comes back with the screen it had, and the machine's fonts are merged and drawn when the finder lands them. Over five seconds with the pointer crossing a pane the viewer ran 60 ms where it ran 2,620, and with nothing happening 20 ms where it ran 2,640, the X server none where it ran 100. After, with the same command as the commit before: | Method | Window | Mean | StdDev | CPU | X server CPU | Drawn | Triangles | Allocated | |--------- |---------- |--------:|---------:|--------:|-------------:|------:|----------:|----------:| | Text | 1100x700 | 1.004 s | 0.0013 s | 0.011 s | - | - | - | 254.53 KB | | Pictures | 1100x700 | 1.005 s | 0.0006 s | 0.004 s | - | - | - | 33.42 KB | | Text | 3840x2160 | 1.005 s | 0.0006 s | 0.005 s | - | - | - | 903.42 KB | | Pictures | 3840x2160 | 1.004 s | 0.0002 s | 0.003 s | - | - | - | 33.42 KB | No frame of the sixty is drawn, the X server does nothing, and a second costs the process 3 to 11 ms where it cost 554 ms to 7.5 s: what is left is the managed side flattening the same screen sixty times, which is where the allocations are. Mean is still a second, which is the wait still being waited, and at 4K it is a second for the first time. NativeFrameBenchmarks is unchanged by this: its frames differ, and each is still drawn, at what it cost before.
… a frame a turn deview.h had it drawing one frame. It is one turn of the window: what the window does not already show is drawn, events are read, and the call comes back when the next frame is due. That it draws nothing for a frame already on the screen is part of what a caller relies on, since it is why the managed side calls it every turn and never says what changed. CMakeLists.txt explained SUPPORT_CUSTOM_FRAME_CONTROL by what EndDrawing did with it on. EndDrawing is no longer called, so nothing rests on that flag, and the busy wait flag beside it is the one that still matters: Rest waits with raylib's WaitTime. And the viewer benchmarks' Program.cs said nothing there opens a window, which the Native classes now do wherever there is a shim and a display.
… turn in claude.md todo.md's fourteen performance items are fixed, so they go, and what each fix says it did not reach takes their place: where a diff stops being minimal, what still rebuilds the queue's list, what a batch's one write means for a snapshot withdrawn during it, the listener table growing with the machine's connections, the half of the zoom range the WinForms head still scales on every paint, what an idle Linux window still does sixty times a second, and for macOS the checks that would confirm on a Mac what nothing here can run. Three of the smaller items move with them. The Linux press and release that arrive together was run in the container rather than left as plausible: none of ten such clicks was seen and three of ten key presses. GetWindowPosition every frame is not the X round trip it was taken for, since raylib 6.0 answers it from what its window position callback last heard, so that item goes. And the macOS picture that lands during a spinner's repaint now has a second case, the scaled copy of an enlarged picture. claude.md says what a turn of the Linux loop is now that it is not a frame on the screen, what a new input to a frame has to be added to, how its spinner keeps turning, and where the head's own benchmarks run.
The figures in the performance commits were taken while other builds were
running. Every benchmark was run again, before and after, one project at a
time on a machine doing nothing else, and the Linux head's two in the
container against the branch as merged. Nothing changed by more than the
noise it was taken in, but six figures that are quoted as what is left, or as
what a paint used to cost, are now the quiet ones:
building a screen of 2,000 entries 0.55 ms and 1.1 MB, not 1.1 and 3.2,
which was the build before the fix
a changed 4K frame of CJK rows 3.6 ms, not 4.6
anchoring 500 call sites 0.7 s, not 955 ms
a 4000 by 3000 pair at 400% 15 ms, not 17
an idle second of the Linux head 3 to 9 ms where it was half a second
to ten, since the merged branch also
skips the managed half of a turn
a paint of long rows, and of a 15 and 24 ms, not 18 and 28
megabyte line, before
SimonCropp
changed the title
Co-authored-by: SimonCropp <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Every bug in
todo.mdfrom the review ofmainat 991bc48, and then every performance item in it: the five bugs in the viewer's model, the twenty seven that six per-area reviews found (library, inline patcher, tray, and the Windows, Linux and macOS heads), and the fourteen performance items across the same areas. One commit per fix from the second round on, so any of them can be reverted alone. In the performance round a benchmark is committed ahead of each fix that replaces a path, so the earlier number can be had again from history.Needs attention before merging
macos-14job is the only build of the four macOS fixes and the two macOS performance changes. It compiles them and its suite passes, but two of the fixes are event handling, which no capture exercises, and neither performance change has been measured: a capture draws no spinner and never takes the scaled copy.todo.mdlists the check that would confirm each on a Mac.native/changed in both rounds, and both ofbuild-native's binaries PRs are merged into this branch: VerifyTests/DiffEngine#923 for the bug fixes and VerifyTests/DiffEngine#924 for the performance changes, built from 28f1528, after whichnative/has not changed. So the committed.soand.dylibare built from the sources beside them. The C ABI is unchanged throughout.InlineStaging.Settle(...)in place ofClearStaged(...)inInlineEngine.Settle(). DiffEngine's half is in here.CreateProcesswith handle inheritance off, so a test run no longer waits on them. That was proven with a console exe, a windowed exe and a.cmdstanding in; the real tools were not run.Round one: the viewer's model (5)
RequiresTarget: false), so a new snapshot gets no placeholder, and the eight map formats EmptyFiles has no template for reach the viewer at all.Round two (27)
Library (3)
UseShellExecute: falseno longer inherit the test host's handles on Windows.Inline patcher (6)
do!,let! x =) is indented from the column the expression starts at, so it compiles.FsCompilerRoundTripTestsnow covers those shapes underdotnet fsi.Snapshotcall goes in front ofConfigureAwait,ToTaskandGetAwaiter, in both languages.Appendpasses over a call in the member that already has aSnapshotcall.Removeofsettings.Snapshot("old");takes the statement, where it used to leavesettings;. Awaited, assigned, returned or passed, only the call goes.InlineStaging.Settleclears only the running one's.Tray (6)
Mainis[STAThread], so Debug view's Copy copies.MoveorDifffor a tracked pair keeps the tool it was tracked with.Windows head (4)
Raiserestores a window minimised from maximised as maximised.Linux head (4), built and run in an
ubuntu:24.04container as theunixjob isawith Shift held, so Caps Lock no longer turns accept into accept-all.macOS head (4, one of them half), compiled and run only by CI
caltandligaare off, so<>and!=are drawn as the characters they are. The five OSX baselines that moved with it were taken again from the job's artifact.Round three: performance (14)
Every performance item in
todo.md, each measured before and after by a benchmark that is now in the repository. The figures are one pass of every benchmark on the tree before this round and on the branch, a project at a time on a machine doing nothing else: BenchmarkDotNet in process, .NET 10, a Ryzen 9 5900X. The Linux head's are from anubuntu:24.04container under Xvfb with Mesa's software rasteriser on four threads, as theunixjob runs it.Viewer model (5)
ScreenCache), and the WinForms head and the native payload both stop at a screen they were handed last frame.Library (1)
Inline snapshots (2)
InlineApplier.ApplyAll), each still with an outcome of its own, and the scan rents its map.InlineStaging.Clearkeeps the list of staging directories for a second rather than walkingobjfor every passing verification.InlineStaging.Clearwith nothing stagedWindows head (2)
Linux head (2), built, run and measured in the container
macOS head (2), compiled and run only by CI, and not measured
CTLines the last two draws made. Which characters can go in one run is asked of the embedded font rather than of four ranges written down, and that half is the model's, so it is measured: segmenting a hundred rows of box drawing went from 116 µs and 319 KB to 15 µs and 3 KB.What each fix left is in
todo.mdunder Performance, with the checks that would confirm the two macOS changes on a Mac.Tests
Each fix has tests that fail without it, except where no test can reach it: keyboard and mouse handling inside the Linux shim was verified by hand in the container with
xdotool, and the macOS event handling not at all. The performance changes have tests for what they must not change (a diff past its budget is still a correct diff, a batch's outcomes are each patch's own, a frame the cache hands back is the frame a build would give) and benchmarks for what they do.Release build is clean, and
dotnet test --solution src/DiffEngine.slnx --configuration Releasepasses on Windows: 3,038 tests, 0 failed, 27 skipped. In the Linux container the branch as it stands builds with no warnings, the viewer's tests with the pixel snapshots on pass 771 with 1 skipped, and all fifteen Linux baselines reproduce byte for byte.Also in here
todo.mdloses every bug and every performance item, and gains what each fix says it did not reach. Of the smaller items, one Linux one turned out not to be a problem and is gone (GetWindowPositionis answered from a cached value in raylib 6.0), and one was run rather than left as plausible.src/DiffEngine.Benchmarks,src/DiffEngineViewer.Benchmarksandsrc/DiffEngineViewer.Windows.Benchmarks.claude.mdsays why three and why in process.docs/andclaude.mddescribe the changed behaviour.