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

Don't emit frozen string literals for rare code paths #78370

Closed
wants to merge 1 commit into from

Conversation

MichalStrehovsky
Copy link
Member

RyuJIT has a concept of string literals used in rare codepaths (think: the string literal with argument name when throwing ArgumentNullException). Accessing these string literals doesn't have to be instant because we're going to do an expensive thing anyway. We didn't implement this before.

My back-of-envelope calculation was suggesting we could save 20 kB for a hello world if we replace frozen string literals (that are fully laid out and aligned the way GC expects at runtime) with WTF-8 data strings that we hydrate into a System.String at runtime when they're needed.

Unfortunately, the actual size savings are mediocre - about 512 bytes to 1 kB on a Hello World, because accessing the lazy string literal requires quite a bit more code:

     static void Main(string[] args)
    {
        if (args.Length == 0)
            throw new Exception("Lol");

        Console.WriteLine("Lol");
    }
G_M24006_IG01:              ;; offset=0000H
       56                   push     rsi
       4883EC20             sub      rsp, 32
                                                ;; size=5 bbWeight=1    PerfScore 1.25
G_M24006_IG02:              ;; offset=0005H
       83790800             cmp      dword ptr [rcx+08H], 0
       7413                 je       SHORT G_M24006_IG04
       488D0D00000000       lea      rcx, gword ptr [(reloc 0x400000000043fd78)]
       E800000000           call     System.Console:WriteLine(System.String)
       90                   nop
                                                ;; size=19 bbWeight=1    PerfScore 5.75
G_M24006_IG03:              ;; offset=0018H
       4883C420             add      rsp, 32
       5E                   pop      rsi
       C3                   ret
                                                ;; size=6 bbWeight=1    PerfScore 1.75
G_M24006_IG04:              ;; offset=001EH
       488D0D00000000       lea      rcx, [(reloc 0x400000000043f700)]
       E800000000           call     CORINFO_HELP_NEWSFAST
       488BF0               mov      rsi, rax
       488BCE               mov      rcx, rsi
       E800000000           call     System.Exception:.ctor():this
       488D0D00000000       lea      rcx, [(reloc 0x400000000043fd70)]
       E800000000           call     CORINFO_HELP_STRCNS_FROM_HANDLE
       488D4E08             lea      rcx, bword ptr [rsi+08H]
       488BD0               mov      rdx, rax
       E800000000           call     CORINFO_HELP_ASSIGN_REF
       488BCE               mov      rcx, rsi
       E800000000           call     CORINFO_HELP_THROW
       CC                   int3

Notice the extra 5 bytes for call CORINFO_HELP_STRCNS_FROM_HANDLE on top of the lea.

Still sending this out for people to have a look, but it's probably a dead end. Oh well.

We could potentially change the helper to accept an integer index, but we will have trouble assigning integer indices to strings because the compiler is multithreaded and compiles methods in arbitrary orders (and we need outputs to be stable).

Cc @dotnet/ilc-contrib

@MichalStrehovsky MichalStrehovsky added the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Nov 15, 2022
@ghost
Copy link

ghost commented Nov 15, 2022

Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas
See info in area-owners.md if you want to be subscribed.

Issue Details

RyuJIT has a concept of string literals used in rare codepaths (think: the string literal with argument name when throwing ArgumentNullException). Accessing these string literals doesn't have to be instant because we're going to do an expensive thing anyway. We didn't implement this before.

My back-of-envelope calculation was suggesting we could save 20 kB for a hello world if we replace frozen string literals (that are fully laid out and aligned the way GC expects at runtime) with WTF-8 data strings that we hydrate into a System.String at runtime when they're needed.

Unfortunately, the actual size savings are mediocre - about 512 bytes to 1 kB on a Hello World, because accessing the lazy string literal requires quite a bit more code:

     static void Main(string[] args)
    {
        if (args.Length == 0)
            throw new Exception("Lol");

        Console.WriteLine("Lol");
    }
G_M24006_IG01:              ;; offset=0000H
       56                   push     rsi
       4883EC20             sub      rsp, 32
                                                ;; size=5 bbWeight=1    PerfScore 1.25
G_M24006_IG02:              ;; offset=0005H
       83790800             cmp      dword ptr [rcx+08H], 0
       7413                 je       SHORT G_M24006_IG04
       488D0D00000000       lea      rcx, gword ptr [(reloc 0x400000000043fd78)]
       E800000000           call     System.Console:WriteLine(System.String)
       90                   nop
                                                ;; size=19 bbWeight=1    PerfScore 5.75
G_M24006_IG03:              ;; offset=0018H
       4883C420             add      rsp, 32
       5E                   pop      rsi
       C3                   ret
                                                ;; size=6 bbWeight=1    PerfScore 1.75
G_M24006_IG04:              ;; offset=001EH
       488D0D00000000       lea      rcx, [(reloc 0x400000000043f700)]
       E800000000           call     CORINFO_HELP_NEWSFAST
       488BF0               mov      rsi, rax
       488BCE               mov      rcx, rsi
       E800000000           call     System.Exception:.ctor():this
       488D0D00000000       lea      rcx, [(reloc 0x400000000043fd70)]
       E800000000           call     CORINFO_HELP_STRCNS_FROM_HANDLE
       488D4E08             lea      rcx, bword ptr [rsi+08H]
       488BD0               mov      rdx, rax
       E800000000           call     CORINFO_HELP_ASSIGN_REF
       488BCE               mov      rcx, rsi
       E800000000           call     CORINFO_HELP_THROW
       CC                   int3

Notice the extra 5 bytes for call CORINFO_HELP_STRCNS_FROM_HANDLE on top of the lea.

Still sending this out for people to have a look, but it's probably a dead end. Oh well.

We could potentially change the helper to accept an integer index, but we will have trouble assigning integer indices to strings because the compiler is multithreaded and compiles methods in arbitrary orders (and we need outputs to be stable).

Cc @dotnet/ilc-contrib

Author: MichalStrehovsky
Assignees: MichalStrehovsky
Labels:

NO-MERGE, area-NativeAOT-coreclr

Milestone: -

@jkotas
Copy link
Member

jkotas commented Nov 15, 2022

Unfortunately, the actual size savings are mediocre - about 512 bytes to 1 kB on a Hello World

Are the working set savings (on Linux) more interesting? Frozen string literals need relocs. These lazy strings should not need relocs.

@EgorBo
Copy link
Member

EgorBo commented Nov 15, 2022

Hm. what is the write barrier doing there? CORINFO_HELP_ASSIGN_REF

@MichalStrehovsky
Copy link
Member Author

Are the working set savings (on Linux) more interesting? Frozen string literals need relocs. These lazy strings should not need relocs.

Do you have a scenario in mind that would be worth measuring?

I guess it could help private working set a bit because we no longer need a reloc within the frozen object. We could potentially also introduce a cold data region where rarely used things like this could be placed. But on the other hand, the increase in code size will hurt working set until we do hot/cold splitting in NativeAOT.

Hm. what is the write barrier doing there? CORINFO_HELP_ASSIGN_REF

I think it's setting the message field on the newed up exception object.

@jkotas
Copy link
Member

jkotas commented Nov 16, 2022

Do you have a scenario in mind that would be worth measuring?

It can be something as simple as Console.WriteLine(Environment.WorkingSet).

@EgorBo
Copy link
Member

EgorBo commented Nov 16, 2022

I think it's setting the message field on the newed up exception object.

Ah, right, it's just that in my branch new Exception(string) is not inlined so the write-barrier is hidden.

@MichalStrehovsky
Copy link
Member Author

Do you have a scenario in mind that would be worth measuring?

It can be something as simple as Console.WriteLine(Environment.WorkingSet).

Looks like it's doing worse. I added the call to the very beginning of the DynamicGenerics test and the numbers look like this for 5 runs (Linux x64 Release):

Before:

7892992
7995392
8097792
8085504
7991296

After:

8159232
7962624
8167424
8019968
8146944

(I also tried the comparison for the "pointer compression" I have in the other pull request while I'm at it and that one doesn't seem to have an effect on working set, which is good and kind of expected based on my mental model. Of course it's still noisy but there's no big outliers like here.)

@MichalStrehovsky
Copy link
Member Author

We can revisit this after we have hot/cold splitting.

@ghost ghost locked as resolved and limited conversation to collaborators Dec 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-NativeAOT-coreclr NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants