Skip to content

Conversation

@dsyme
Copy link
Contributor

@dsyme dsyme commented Feb 12, 2022

Addresses #5457 by switching F# Interactive for .NET Core to a model where we emit a separate assembly for each interaction entered into the system. These assemblies are emitted using ILWrite generating bytes (and, optionally, bytes for debug symbols). This is the same model used by C# Interactive.

The successful generation of debug information for scripts can be seen below, where a stack for an exception from scripting code now shows the line number correctly (note the crucial in C:\GitHub\dsyme\fsharp\a.fsx:line 3 for the failure point)

a.fsx:

let f() = 
    printfn "hello"
    failwith "abc"
    printfn "hello"

f()

Running as a single script:

>artifacts\bin\fsi\Debug\net472\fsi --debug --optimize- a.fsx
hello
System.Exception: abc
   at FSI_0001.f() in C:\GitHub\dsyme\fsharp\a.fsx:line 3
   at <StartupCode$FSI_0001>.$FSI_0001.main@()
Stopped due to error

Running as pipe-to-stdin input:

>artifacts\bin\fsi\Debug\net472\fsi --debug --optimize- < a.fsx

Microsoft (R) F# Interactive version 12.0.0.0 for F# 6.0
Copyright (c) Microsoft Corporation. All Rights Reserved.

For help type #help;;

> hello
System.Exception: abc
   at FSI_0002.f() in C:\GitHub\dsyme\fsharp\stdin:line 3
   at <StartupCode$FSI_0002>.$FSI_0002.main@()
Stopped due to error

The generation of multiple assemblies is done purely in fsi.fs so as not to inflict this on the rest of the codebase (separately, fsi.fs should really be split into multiple files). As far as the F# language and most of the compiler is concerned, the code being entered into FSI remains as one logical assembly, though see below with regard to the impact of this.

The generated assemblies are called

  • FSI-ASSEMBLY1
  • FSI-ASSEMBLY2
  • ...

Compat switch

There is a switch fsi --multiemit that turns off or on the use of multi-assembly emit. It is on by default for .NET Core. When off, this reverts to Reflection Emit for a single-dynamic-assembly generation. This is off by default for .NET Framework for compat reasons (and also there is no problem with generating debug information for refemit there)

Internals and accessibility

Generating into multiple assemblies raises issues for some things that are assembly bound such as "internals" accessibility. In a first iteration of this we had a failing case here:

artifacts\bin\fsi\Debug\net472\fsi --debug --optimize-

// Fragment 1
> let internal f() = 1;;
val internal f: unit -> int

// Fragment 2 - according to existing rules it is allowed to access internal things of the first
f();; 
System.MethodAccessException: Attempt by method '<StartupCode$FSI_0003>.$FSI_0003.main@()' to access method 'FSI_0002.f()' failed.
   at <StartupCode$FSI_0003>.$FSI_0003.main@()

This is because we are now generating into multiple assemblies.

According to the current F# scripting programming model (the one checked in the editor), the "internal" thing should be accessible in subsequent fragments. Should this be changed? No:

  • It's very hard to adjust the implementation of the editor scripting model to consider fragments delimited by ;; to be different assemblies, whether in the editor or in F# Interactive.

  • And would we even want to? It's common enough for people to debug code scattered with "internal" declarations.

  • In scripts, the ;; aren't actually accurate markers for what will or won't be sent to F# Interactive, which get added implicitly.

    For example, consider the script

    let internal f() = 1;;
    f();; 

    In the editor should this be given an error or not? That is, should the ;; be seen as accurate indicators of separate script fragments? (Answer: yes if we know the script will be piped-to-input, no if the script is used as a single file entry - when the ;; are ignored)

  • Further, this would be a breaking change, e.g. it could arise in an automated compat situation if people are piping into standard input and the input contains ;; markers.

So this is actually tricky to fix.

Because of this we emit IVTs for the next 30 FSI-ASSEMBLYnnn assemblies on each assembly fragment, giving a warning when an internal thing is accessed across assembly boundaries within that 30 (reporting it as a deprecated feature), and give an error if internal access happens after that.

From a compat perspective this seems reasonable, and the compat flag is available to return the whole system to generate-one-assembly behaviour.

Note there is general assumption in this that on modern dev machines (where users execute multiple interactions) then generating 50, 100 or 1000 or 10000 dynamic assemblies by repeated manual execution of code is not a problem: the extra overheads of multiple assemblies compared to one dynamic assembly is of no real significance in developer REPL scenarios given the vast amount of memory available on modern 64-bit machines.

Debugging .NET Core F# Interactive in Visual Studio

There was one bit of work relating to "Debug F# Interactive" feature in Visual Studio. This feature was previously connecting to the wrong process - the we use to start the F# Interactive starts a subordinate process

Process1: dotnet fsi

Process 2: dotnet.exe c:\Program Files\....\dotnet.dll ...

The "Debug F# Interactive" feature in Visual Studio now connects to the second process, not the first, for debugging. To do that we need to know its process ID. In this case we get the second process to write out the process ID to a file first thing by feeding it the code to do that as first input.

@smoothdeveloper
Copy link
Contributor

@dsyme, if there is a chance Roslyn does the same with their "interactive" core, maybe it will lead to a path, where each evaluation, can be written in any .net languages (there are many out there)?

If proper "interactive" "compiler" "runtime" infrastructure is put together, I'm sure it will eventually happen.

@dsyme
Copy link
Contributor Author

dsyme commented Feb 13, 2022

@dsyme, if there is a chance Roslyn does the same with their "interactive" core, maybe it will lead to a path, where each evaluation, can be written in any .net languages (there are many out there)?

For this kind of functionality I suggest using .NET Interactive polyglot notebooks. It's not something that will ever be covered by the F# Interactive REPL

@dsyme
Copy link
Contributor Author

dsyme commented Feb 13, 2022

I implemented a check for access to internal things defined in previous F# interactive interactions, currently testing it manually, see below

image

Currently the implementation emits IVTs to the next 30 dynamic assemblies for each fragment. If you insist on continuing to access the internal thing beyond that you get an error as warned:

image

I also added a flag to revert to using a single dynamic assembly:

--dynamicassembly[+|-]                   Use a single dynamic assembly

@dsyme
Copy link
Contributor Author

dsyme commented Feb 13, 2022

There is one remaining bit of work relating to "Debug F# Interactive" feature in Visual Studio:

This feature is currently connecting to the wrong process. The dotnet fsi we use to start the F# Interactive starts a subordinate process

`dotnet.exe c:\Program Files\....\dotnet.dll ...`

The "Debug F# Interactive" feature in Visual Studio should connect to the second process, not the first, for debugging. But to do that we need to know its process ID (I'm not really sure how to do that). Either that or we need to simply start the second process directly ourselves.

@dsyme dsyme changed the title [WIP] Multi-assembly+debug emit for FSI Multi-assembly+debug emit for FSI Feb 14, 2022
@dsyme
Copy link
Contributor Author

dsyme commented Feb 14, 2022

This is ready for review @KevinRansom @TIHan @vzarytovskii

@heronbpv
Copy link

heronbpv commented Feb 15, 2022

Just checking if I understood it correctly: the accessibility on internal members/binds is only a problem if, and only if, we declare them at different dispatches to FSI? So, albeit less affordable, as long as you delimited the usage to only the batch in which it's dispatched, it's ok and no error should arise?

@dsyme
Copy link
Contributor Author

dsyme commented Feb 15, 2022

Just checking if I understood it correctly: the accessibility on internal members/binds is only a problem if, and only if, we declare then at different dispatches to FSI? So, albeit less affordable, as long as you delimited the usage to only the batch in which it's dispatched, it's ok and no error should arise?

Yes, that is correct.

})

do if usingNetCore then
inputQueue.Post($"""System.IO.File.WriteAllText(@"{trueProcessIdFile}", string (System.Diagnostics.Process.GetCurrentProcess().Id));;""")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is used in Visual Studio to get the process ID from F# Interactive

// LINE3 --> SERVER-PROMPT>
//
// We skip these lines.
if usingNetCore && not seenPidJunkOutput && line.StartsWith("val ") then
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is used to filter the output that arises from getting the process ID from the F# Interactive session

[<Test>]
let ``printing-default-stdout-50 --langversion:5_0`` () =
printing "--langversion:5.0" "z.output.test.default.stdout.50.txt" "z.output.test.default.stdout.50.bsl" "z.output.test.default.stderr.txt" "z.output.test.default.stderr.bsl"
let ``printing-default-stdout-50 --langversion:5_0 --refemit+`` () =
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This adds some testing for refemit on and off. The "printing" test is fed into F# interactive fragment by fragment so is a useful place to locate any testing that is related to piece-by-piece evaluation of interactive fragments




stdin(0,1): warning FS2303: Accessing the internal type, method or field 'f' from a previous evaluation in F# Interactive is deprecated and may cause subsequent access errors. To enable the legacy generation of a single dynamic assembly that can access internals, use the '--refemit' option.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a the line that gets output for the "access internals" warning. THe rest of this file is a duplicate of the standard output from the printing tests

System.DayOfWeek.Tuesday
;;

let internal f() = 1;; f();; // should give a warning in multi-assembly interactive emit
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a test for accessing internals

open FSharp.Compiler.AbstractIL.IL
open FSharp.Compiler.AbstractIL.ILBinaryReader
open BenchmarkDotNet.Attributes
open BenchmarkDotNet.Running
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file is all cleanup

@dsyme
Copy link
Contributor Author

dsyme commented Feb 15, 2022

@vzarytovskii I notice the name of the flag in my PR "--refemit" conflicts a bit with the "--refonly" from yours for reference assemblies, with entirely different meanings

Not sure of a better name for mine. The flag means this: run FSI in a legacy mode where it generates a single dynamic assembly using reflection emit and allows internal access across multiple fragments without complaint

@dsyme
Copy link
Contributor Author

dsyme commented Feb 15, 2022

I notice the name of the flag in my PR "--refemit" conflicts a bit with the "--refonly" from yours for reference assemblies, with entirely different meanings. Not sure of a better name for mine. The flag means this: run FSI in a legacy mode where it generates a single dynamic assembly using reflection emit and allows internal access across multiple fragments without complaint

I'll change to --refemit to --multiemit, on by default for .NET Core

@dsyme
Copy link
Contributor Author

dsyme commented Feb 15, 2022

@jonsequitur @brettfo Note this should give better debug stacks for F# Interactive notebooks, and adds the potential to have breakpoints for notebooks.

@auduchinok
Copy link
Member

auduchinok commented Feb 16, 2022

Because of this we emit IVTs for the next 30 FSI-ASSEMBLYnnn assemblies on each assembly fragment, giving a warning when an internal thing is accessed across assembly boundaries within that 30 (reporting it as a deprecated feature), and give an error if internal access happens after that.

@dsyme Would it work if these assemblies had the same name? Would it be possible to add a single IVT for something like FSI-ASSEMBLY then? The compiled types would still have different FQNs if each interaction is wrapped in a separate top level module, so the only question is can we generate multiple assemblies with the same name and resolve symbols correctly.

@dsyme
Copy link
Contributor Author

dsyme commented Feb 16, 2022

@dsyme Would it work if these assemblies had the same name? Would it be possible to add a single IVT for something like FSI-ASSEMBLY then? The compiled types would still have different FQNs if each interaction is wrapped in a separate top level module, so the only question is can we generate multiple assemblies with the same name and resolve symbols correctly.

Hmmm... I have no experience working in .NET execution contexts where multiple assemblies have the same name. It's possible it's worth a go because the use of name-based reflection over these assemblies is very limited.

@dsyme
Copy link
Contributor Author

dsyme commented Feb 16, 2022

I found a bug in this code when running a more complicated script (a case of accessing internals leaked by optimization). Looks like we need more testing....

@dsyme dsyme merged commit 45518d2 into dotnet:main Feb 23, 2022
@nosami
Copy link
Contributor

nosami commented Mar 1, 2022

Wish this had been around a few years back when I was working on F# support for Xamarin Workbooks. The C# side of things was using the Roslyn model at the time and I had to emulate it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Theme-Simple-F# A cross-community initiative called "Simple F#", keeping people in the sweet spot of the language.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants