Skip to content
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

Some maintenance question #399

Closed
Thorium opened this issue Nov 14, 2023 · 1 comment
Closed

Some maintenance question #399

Thorium opened this issue Nov 14, 2023 · 1 comment

Comments

@Thorium
Copy link
Member

Thorium commented Nov 14, 2023

This is not so much of an issue, but just a maintenance question...

The Utils module has lot of stuff that I think is pre-ValueOption and UOption could be replaced with VOption.
https://github.com/fsprojects/FSharp.TypeProviders.SDK/blob/master/src/ProvidedTypes.fs#L36

I think the utils themselves could also benefit of Spans, for example:
https://github.com/fsprojects/FSharp.TypeProviders.SDK/blob/master/src/ProvidedTypes.fs#L55
...could work something like:

    let splitNameAt (nm:ReadOnlySpan<char>) idx =
        if idx < 0 then failwith "splitNameAt: idx < 0";
        let last = nm.Length - 1
        if idx > last then failwith "splitNameAt: idx > last";
        (nm.Slice(0, idx)), 
        (if idx < last then nm.Slice (idx+1, last - idx) else ReadOnlySpan.Empty)

    let splitILTypeName (nm:ReadOnlySpan<char>) =
        match nm.LastIndexOf '.' with
        | -1 -> UNone, nm
        | idx -> let a, b = splitNameAt nm idx in USome a, b

I would also like to make the DUs without any fields (or only fields of plain non-recursive other DUs) as structs, e.g. here:
https://github.com/fsprojects/FSharp.TypeProviders.SDK/blob/master/src/ProvidedTypes.fs#L2737
https://github.com/fsprojects/FSharp.TypeProviders.SDK/blob/master/src/ProvidedTypes.fs#L2952
https://github.com/fsprojects/FSharp.TypeProviders.SDK/blob/master/src/ProvidedTypes.fs#L2973

Why do I even care, is because with a bigger F# solution with multiple type-providers they end-up taking quite a bit of resources, making VS a bit slow, for example this is a part of memory snapshot of VS2022:
image

@Thorium
Copy link
Member Author

Thorium commented Nov 16, 2023

I tried to change those DUs to structs (master...Thorium:FSharp.TypeProviders.StarterPack:more-structs),
and run some memory & performance analysis with some type-providers and VS, but the difference was within mean square error margin. So no justification for the change.

@Thorium Thorium closed this as completed Nov 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant