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

Optimize GDScript VM codegen for MSVC #81200

Merged
merged 1 commit into from
Sep 25, 2023

Conversation

RandomShaper
Copy link
Member

This allegedly leads to less optimal code than in the case of GCC/Clang's computed gotos, but is an improvement for those building with MSVC. I haven't benchmarked myself, so TIWAGOS.

@Calinou
Copy link
Member

Calinou commented Aug 31, 2023

That reminds me, could we use likely()/unlikely() hints with GCC/Clang here? We do this in error macros already.

@RandomShaper
Copy link
Member Author

That reminds me, could we use likely()/unlikely() hints with GCC/Clang here? We do this in error macros already.

If you mean using them for dispatching opcodes, there's the computed gotos optimization already in place for them, for which there's no point in mixing with (un)likely. Or are you talking about other spot?

@akien-mga
Copy link
Member

Would be interesting to check how this affects build time for this file, both with and without exceptions enabled (see #80513).

Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

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

Looks good to me.

Would like to have some numbers so we know exactly what we gain, but it's not blocking.

@akien-mga akien-mga merged commit 7a29189 into godotengine:master Sep 25, 2023
14 checks passed
@akien-mga
Copy link
Member

Thanks!

@RandomShaper RandomShaper deleted the gd_vm_msvc branch September 25, 2023 15:47
@Faless
Copy link
Collaborator

Faless commented Sep 25, 2023

For posterity, I did run some benchmarks on this.

The MSVC build is much faster with this patch but still noticeably slower than the GCC/MinGW version using the synthetic test in #11518 adapted to 4.x ( test.zip ).

Here are the hyperfine results for 549fcce (+ patch) built with target=template_release production=yes

$ hyperfine.exe -w 1 -r 5 .\test\msvc_base\test.exe .\test\msvc_opt\test.exe .\test\mingw\test.exe
Benchmark 1: .\test\msvc_base\test.exe
  Time (mean ± σ):     22.410 s ±  0.312 s    [User: 22.251 s, System: 0.040 s]
  Range (min … max):   22.075 s … 22.911 s    5 runs

Benchmark 2: .\test\msvc_opt\test.exe
  Time (mean ± σ):     16.885 s ±  0.078 s    [User: 16.701 s, System: 0.052 s]
  Range (min … max):   16.774 s … 16.960 s    5 runs

Benchmark 3: .\test\mingw\test.exe
  Time (mean ± σ):     15.021 s ±  0.103 s    [User: 14.892 s, System: 0.021 s]
  Range (min … max):   14.915 s … 15.194 s    5 runs

Summary
  .\test\mingw\test.exe ran
    1.12 ± 0.01 times faster than .\test\msvc_opt\test.exe
    1.49 ± 0.02 times faster than .\test\msvc_base\test.exe

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

Successfully merging this pull request may close these issues.

5 participants