Repository navigation
Conversation
…sprojects#452) The dynamic path (used in FSI and whenever the compiler does not lower the CE to a static state machine, e.g. Debug builds with SDK 10.0.4xx, fsprojects#473) only worked when every await completed synchronously. In dynamic code, ResumableCode.Yield() resumes with a continuation that just returns true, so code after the yield point in the library's own combinators never ran: - Bind (Task, Async, ^TaskLike): the rest of the computation was dropped and GetResult was never called (truncated sequences, swallowed exceptions, hangs in loops) - WhileAsync: the awaited MoveNextAsync result was discarded and MoveNextAsync was called again (hang) - TaskExtensions.WhileAsync (task { for x in taskSeq }): same, plus it called AwaitUnsafeOnCompleted itself instead of setting ResumptionData, crashing the process with a NullReferenceException These now have explicit `if __useResumableCode` dynamic branches that install a real continuation; the static branches are unchanged. TaskSeqDynamic.DisposeAsync read this._machine.Data inside a task { }, which the F# compiler miscompiles (it substitutes every ResumableStateMachine<_> instantiation with the task's own struct). The field is now read before entering the task. Also fixes the duplicated last item of fsprojects#452 on the static path: when the source's DisposeAsync completes asynchronously, the TryFinallyAsync compensation suspended without clearing `current`. Adds TaskSeq.Dynamic.Forced.Tests.CE.fs, which forces the dynamic path with a non-inline Run and awaits that really suspend. Fixes fsprojects#505
Instead of separate `if __useResumableCode` branches, Bind (Task, Async, ^TaskLike) is now Combine(<await step>, <continuation>), where the await step ends in a tail-position ResumableCode.Yield(). In static code the resume falls through Combine into the continuation; in dynamic code CombineDynamic installs it. The continuation is still invoked in one place only, so it is inlined once. Yield now sets `current`/`awaiter` before yielding and yields in tail position, so nothing has to run after the resume. This makes it behave the same in static and dynamic code (the post-resume code used to be skipped in dynamic code, and re-set a stale `current` in static code). The number of static state machines is unchanged; MoveNext IL of every changed machine shrank (library -2.1% Release, -3.8% Debug).
Contributor
|
Wow great work |
dsyme
approved these changes
Nov 9, 2026
The static branch of WhileAsync awaited a pending condition (the MoveNextAsync of `for .. in` over an IAsyncEnumerable) via AsTask(), which allocates a Task when the ValueTask is backed by an IValueTaskSource, as a taskSeq's own MoveNextAsync is. It now awaits through the ValueTask's own awaiter, as the dynamic branch already did. The synchronous path is unchanged. Benchmark (for .. in over a source that suspends on each of 100 items): 35.3 KB -> 26.9 KB allocated per op, 80.5-81.9 us -> 74.6-79.2 us.
majocha
marked this pull request as ready for review
November 9, 2026 16:40
Author
|
It's all prompted. I remember a year ago agents couldn't handle resumable code at all. Scary how fast AI progresses. |
This branch has not been deployed
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
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.
Fixes #505. Fixes #473.
The dynamic resumable code path of
taskSeqonly worked when every await completed synchronously. That path is used in FSI and whenever the F# compiler does not lower the CE to a static state machine. With SDK 10.0.4xx that includes Debug builds, silently and without an FS3511 warning, which is the Debug-only truncation reported in #473. Details and repros are in #505.This also contains the likely root cause and fix of the duplicated last item in #452 (static path). It is reproduced offline, but not verified under Orleans.
The underlying problem
In dynamic code,
ResumableCode.Yield()resumes with a continuation that just returnstrue. Code that follows the yield point inside the same delegate is therefore skipped on resume. In static code it runs, because resuming jumps back into the middle of the flattenedMoveNext.BindandWhileAsyncrelied on that code running, so the dynamic path lost everything after the first real await.Changes
TaskSeqBuilder.fsBindandYield: one code path for static and dynamic code, with noif __useResumableCodebranches.Bind(Task, Async,^TaskLike) isCombine(<await step that ends in a tail-position Yield()>, <continuation>). In static code, the resume falls throughCombineinto the continuation as before. In dynamic code,CombineDynamicinstalls the continuation. The continuation is invoked in one place only, so it is inlined once.Yieldsetscurrent/awaiterbefore yielding and yields in tail position, so nothing needs to run after the resume.WhileAsync: keeps separate static and dynamic branches. The dynamic branch installs the continuation explicitly. A single-path version was tried and measured about 10% slower on static loops over synchronously completing sources: the condition'sValueTaskwould have to live in a state-machine field instead of a stack local.MoveNextAsyncthrough theValueTask's own awaiter instead ofAsTask(). This saves aTaskallocation per suspension.TaskSeqDynamic.DisposeAsync: reads_machine.Databefore entering itstask { }. Inside the task, the F# compiler substitutes everyResumableStateMachine<_>instantiation with the task's own struct, so the read returned garbage: skippedfinallyblocks, orEntryPointNotFoundException. This is an F# compiler bug; a fix is being prepared for dotnet/fsharp.TryFinallyAsynccompensation now clearscurrentwhen it suspends on an asyncDisposeAsync. Before,MoveNextAsyncreported the last item a second time.TaskExtensions.fs:WhileAsyncfortask { for x in taskSeq }gets a dynamic branch that follows FSharp.Core's dynamic task protocol (ResumptionData+ResumptionFunc). It used to crash the process with aNullReferenceException.TaskSeq.Dynamic.Forced.Tests.CE.fs(new): 25 tests that force the dynamic path with a non-inlineRun, using awaits that really suspend, plus static-path tests for Wrapping combinators (TaskSeq.map, taskSeq { for .. }) yield the final item twice over an external IAsyncEnumerable on a non-default TaskScheduler (Orleans grain) #452. The existingtaskSeqDynamictests are compiled statically and never exercised the dynamic path.No public API changes.
Testing
Full test suite on the final code (5479 tests; the 2 skipped tests were already skipped before this change):
main, local buildmain: 109 failures in the first 747 tests, then the test host crashes)dotnet fantomas . --checkis clean.Static code size and performance
Static state machines, compared with this PR's first commit (whose static code is
main's) and checked with the local compiler (Debug and Release) and SDK 10.0.114 (Release):MoveNextgrew.MoveNextIL is 2.6% smaller in Release and 4.1% smaller in Debug.Benchmarks: BenchmarkDotNet 0.15.8, in-process, 15 iterations, 3 alternating runs per comparison.
yieldloops,let!on completedTask/ValueTask,do! Task.Yield(),for .. inover synchronously completing sources, orTaskSeq.map+sum.for .. inover a source that suspends on every item (100 items): allocations go from 35.3 KB to 26.9 KB per op, and it is 3–7% faster. This is theAsTask()change.let!allocates about 200 B more than with a separate dynamic branch (Combine's delegates), and is about 5–10% slower. I accepted that, because the dynamic path is only a fallback.Notes for review
taskSeqcode nested inside a statically compiledtask { }can't be tested at the moment. The dotnet/fsharpmaincompiler rejects it with FS3401, and SDK 10.0.114 emits invalid IL. The new tests therefore create their sequences in ordinary functions; see the comment at the top of the test file.ResumableCode.While's dynamic implementation recurses once per iteration that completes synchronously, astaskdoes. A 10,000-iteration loop is tested.🤖 Generated with Claude Code