-
Notifications
You must be signed in to change notification settings - Fork 818
More string
optimizations
#18546
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
More string
optimizations
#18546
Conversation
❗ Release notes required
|
18062d5
to
5eded0b
Compare
5eded0b
to
0c48a37
Compare
277f5c4
to
2e83e22
Compare
tests/FSharp.Compiler.ComponentTests/EmittedIL/StaticOptimizations/String_Enum.fs.il.bsl
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this addition and we can uplift what you called type marker hack
into a more explicit (new) mechanism - a feature toggle mechanism to communicate between compiler and statically optimized library code.
Testing for new compiler + new fsharp core is well covered with existing FSharp.Core tests.
I think it would be wise to add a smoke test for showing that new compiler + old fsharp.core works fine.
(we could also brainstorm on a good way to test the last combination, old compiler + new fsharp.core. A way could be to run fsharp.core test suite, with freshly built Fsharp.Core, using the last-known-good SDK, without compiler overrides)
This special-casing allows us to update FSharp.Core to avoid boxing when caling the `string` function on enums and signed integral types going forward while still allowing the updated version of FSharp.Core to be fully compatible with older compilers. Adding support for some form of constraint in library-only static optimizations instead would have been problematic for multiple reasons. Supporting something like `when 'T : enum<'U>` would have required additional modifications to the compiler and would not have been consumable by older compilers. It would also introduce a new type variable. While something like `when 'T : 'T & #Enum` is already syntactically valid, it would add that constraint to the entire `string` function without further modification to the typechecker. It would also not be consumable by older compilers. I think adding a special case for enums is justifiable since (1) enums are a special kind of type to begin with, and (2) static optimization constraints are only allowed in FSharp.Core, so the change to the language itself is quite small.
7f2499f
to
d5dc4a5
Compare
d5dc4a5
to
4d198f5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good now.
It now makes the code clear about a new construct - CompilerServices.Supports..
- which we can use to communicate between Fsharp.Core and the compiler.
The proto
process covered via CI also ensures that:
- LKG compiler can built this code in fslib
- LKG compiler can built project that references such built fslib (when LKG builds proto)
Description
(This might need an RFC? It also might be too much of a hack to accept... But it does work.)
This PR follows and improves upon #9549. It improves the
string
function's implementation for signed integer types (sbyte
,int16
,int32
,int64
) and enum types based on signed integer types by directly calling the appropriateToString
overload on the underlying type. The boxing and casting of the previous implementation (see #1714, #9153) are now avoided altogether when the type is known at compile-time. All existing culture-invariant and culture-dependent behavior is preserved.This is done in a backwards- and forwards-compatibile way by working within the confines of the existing
when 'T : …
library-only static optimization construct, avoiding the need to extend that feature with new syntax or constraints (as suggested in #9594) or to introduce a new construct to the pickling format. That is, this and newer compilers will still be able to compile code using older FSharp.Core versions, while older F# compilers will be able to consume this and newer FSharp.Core versions without any compile-time or runtime breaking changes.Example of IL before
Example of IL after
Changes
SupportsWhenTEnum
—is added to theMicrosoft.FSharp.Core.CompilerServices
namespace in FSharp.Core. This type is marked[<Sealed; AbstractClass>]
, has no members or constructors, and is hidden by default via[<CompilerMessage(…)>]
. (Could/should we useIsError = true
, and/or useEditorBrowsable
orExperimental
to discourage use from other languages as well?)when 'T : Enum
library-only static optimization constraint and treats it very similarly to the already-possiblewhen 'T : 'T & #Enum
, only the subtype constraint is not propagated back to the outer'T
.when 'T : SupportsWhenTEnum
library-only static optimization constraint. This enables compilers that understand it to process any following static optimization constraints in a different order from compilers that do not understand it while remaining fully compatible with them.string
operator is updated to use a new ordering via thewhen 'T : SupportsWhenTEnum
andwhen 'T : Enum
constraints when compiled with newer compilers. Older compilers will not recognize the new constraints; they will simply skip over them and use the old sequence of constraints exactly as before.The compiler change could be put behind a language feature if necessary.
Tradeoffs
Pros
string
on very common types likeint
.Cons
Alternatives
'T
(and explicitly add the necessary constraints to the core library functions that currently depend on this propagation). This would allow existing syntactically-valid generic constraints to be used (likewhen 'T : 'T & #Enum
) instead of adding a special case forwhen 'T : Enum
.If 2. or 3. had been done in F# 1.0 (or whenever static optimization constraints were added), that would have been ideal. However, doing either 2. or 3. now would involve a significant amount of engineering work, and both would have compatibility problems.
Checklist
Benchmarks
string
on such values for signed integer types (sbyte
,int16
,int32
,int64
) now results in zero allocations and a ~3× speedup.string
on negative and non-cached positive signed integers.string
on enum values, including negative values that do not correspond to a member of the given enum type.Source