Transparent compiler: test that TC reuses check results after project modifications

[auduchinok]

Since a lot of things is being redone in how results are cached and invalidated, we could also try to add some other improvements.

When changing a project by adding a new file or reordering existing ones, everything in this project is invalidated. We could, however, preserve results up to the point where there's an observable change for the analysis.

Consider this project:

1.fs
2.fs
3.fs
4.fs
Program.fs

Adding a file after 4.fs could, in theory, preserve cached results for almost the whole project, and the new file would be analyzed instantly, compared to having to analyze the first 4 files first, as things stand now. Reordering files like 3.fs and 4.fs could also allow keeping cached results for the first two files.

I think this would significantly improve user experience on bigger projects.

[vzarytovskii]

Since a lot of things is being redone in how results are cached and invalidated, we could also try to add some other improvements.

When changing a project by adding a new file or reordering existing ones, everything in this project is invalidated. We could, however, preserve results up to the point where there's an observable change for the analysis.

Consider this project:

1.fs
2.fs
3.fs
4.fs
Program.fs

Adding a file after 4.fs could, in theory, preserve cached results for almost the whole project, and the new file would be analyzed instantly, compared to having to analyze the first 4 files first, as things stand now. Reordering files like 3.fs and 4.fs could also allow keeping cached results for the first two files.

I think this would significantly improve user experience on bigger projects.

This is what happens now, TC results are reused for parts of graph which aren't changed.

[auduchinok]

This is what happens now, TC results are reused for parts of graph which aren't changed.

Do you mean in the Transparent Compiler? I haven't seen this in the previous implementation, i.e. any change to project options would invalidate the whole project.

[vzarytovskii]

This is what happens now, TC results are reused for parts of graph which aren't changed.

Do you mean in the Transparent Compiler? I haven't seen this in the previous implementation, i.e. any change to project options would invalidate the whole project.

Yeah, in the transparent compiler, old one didn't change.

[auduchinok]

Yeah, in the transparent compiler, old one didn't change.

Oh, that's really cool! 🔥 Thanks for answering @vzarytovskii

[auduchinok]

@0101 Could you point me to the tests that verify this behavior, please, if there are any?

[0101]

Not sure they're there, but should be easy to add, based on e.g. this one: https://github.com/dotnet/fsharp/blob/be658c56fe5436b6b3dd8ac291ef118f8f9cc4eb/tests/FSharp.Compiler.ComponentTests/FSharpChecker/TransparentCompiler.fs#L244C1-L265C125

Just add a addFileAbove in the workflow and check that files above the new file won't be re-checked. (That would model a situation where previously Options would change. With Snapshot it's changed by every keystroke so that is covered by the existing test already.)

It should not be flaky btw, if it is it needs to be fixed.

[auduchinok]

OK, so I'll open the issue back until we have some guaranties here guarded by tests.

[vzarytovskii]

OK, so I'll open the issue back until we have some guaranties here guarded by tests.

It'll likely not be covered for some time. All transparent compiler tests are disabled, since they are too flaky currently (and since the feature is highly experimental and unstable).

Also, could you please rename the issue, so it's about tests?

[0101]

All transparent compiler tests are disabled, since they are too flaky currently (and since the feature is highly experimental and unstable).

Actually majority of them are running and stable (all tests using FSharpChecker that we already had). And most of the ones you disabled were also not flaky. It was mainly a newly added test by dawe that was written incorrectly and was since deleted. I think only the fuzzing test remained flaky and everything else should be re-enabled.

[vzarytovskii]

All transparent compiler tests are disabled, since they are too flaky currently (and since the feature is highly experimental and unstable).

Actually majority of them are running and stable (all tests using FSharpChecker that we already had). And most of the ones you disabled were also not flaky. It was mainly a newly added test by dawe that was written incorrectly and was since deleted. I think only the fuzzing test remained flaky and everything else should be re-enabled.

I think I disabled all of them, since needed CI to be green due to 1es deadline, and just carpet-disabled all of them

[0101]

This seems to still be running, which has all component tests running with TC and all other tests that use FSharpChecker also.

[vzarytovskii]

This seems to still be running, which has all component tests running with TC and all other tests that use FSharpChecker also.

I mean I put skip on TC-only tests