Large binary size on release

[bhgsbatista]

Hello!

I just tried out compiling the Hello World example, which was a surprisingly smooth process, so props for that!

However, when I inspected the binary that was generated on Windows, it was 400+ KB. That surprised me since it was a small "Hello World" program. I ran cargo in release mode cargo build --release and got the binary down to 245 KB which is still quite large.

I decided to investigate the hello_world.dll binary using snowman disassembler and Strings utility what I discovered is that there's a large chunk of error text that gets inserted into the binary (approx. 64KB), containing a TON of error messages like this:

API function missing: godot_gdnative_core_api_struct.godot_color_new_rgba
API function missing: godot_gdnative_core_api_struct.godot_color_new_rgb
...

Here is the output of the Strings tool: https://gist.github.com/bhgsbatista/0785a806052f247c9b2e72dc8583366f

Basically for every API function we link against, we inject this error message if it's not found. Seems like something we should skip doing for release builds.

[karroffel]

Hey! I agree that this could be improved. Maybe in order to remove the size of the strings we can use .or_else(|| panic!("API function missing: {}.{}", $api_name, $func_name)) instead of the .unwrap(...)

That should cut down the size of the individual strings a bit, but I think for debugging purposes this is still good to have.

In theory none of those function pointers should ever be null, so maybe we can indeed make this a debug-only feature.

In theory none of those function pointers should ever be null, but in practice -- remember the Android API struct?

As for the issue's topic, it should be noted that Rust binaries can easily get as large as ~50 megabytes once you start doing anything real, since everything is static linked in Rust. The impact of our debug strings is in reality very small compared to Rust binary sizes in general.

If you want the compiler to optimize for size, please see the relevant documentation on how to do so. Please keep in mind that this will probably have a significant impact on performance. Otherwise, I suggest marking this issue as won't fix, since this is really a general problem with Rust and there's nothing significant we can do.

[karroffel]

You are right, however usually people distribute the native binaries alongside a Godot version they picked, so if this does not happen with a debug build then it should be fine to disable those checks for release builds, no?

True, but there are two things to consider here:

1. Rust fn pointers are always non-zero. We have to go out of our way to write unsafe code to unwrap the Options without checking if they're zero.
2. This is a library for game development. In games, binary size is usually dwarfed by assets. 64 KB is at most a few icons in today's HiDPI world. The supposed benefit is so negligible that it's literally not worth giving up anything for, even if it's just a small runtime check that will almost never fail, moreso if avoiding the check means getting more transmutes into the codebase.

Given the points above, I'd oppose the removal of these checks without a much more compelling reason.

[karroffel]

On mobile devices code size can also become a problem, but the essence of your point is correct. I still think we can trim down size without any usability cost or change of functionality by using lazy panics with a format string. But I agree it's not urgent.

I'd imagine the repetitive strings will just get zipped away without any extra complexity on our side, anyway. Formatting at runtime also inserts more formatting code. I wonder if it will actually reduce (zipped) binary size.

[karroffel]

That's fair. I guess we can let this one be unchanged unless we get more feedback that this is somehow a critical problem.

[bhgsbatista]

I have a web development background, where code size matters a lot (for both bandwidth reasons, as well as cost of loading and executing the code on mobile devices). You could argue that Godot is not a web focused game engine, which is true, but it could be a great choice for developers targeting mobile web browsers (if that ever happens, other people might want to revisit this issue).

I also know C++ where Macros and conditional compilation is prominent, so it seemed strange to me that we don't conditionally compile away a lot of this error checking code for release builds.

I don't know Rust very well at all, so if you say changing this behavior will have knock on effects on other parts of the codebase, then I'd agree this is not worth changing.

bandwidth reasons

Websites conscious about bandwidth should run compression on their web servers. I'd imagine the repetitive strings will just get zipped away. Can you confirm that there is a significant size difference after compression?

I also know C++ where Macros and conditional compilation is prominent, so it seemed strange to me that we don't conditionally compile away a lot of this error checking code for release builds.

I'm not sure if this has any bearings on the Rust bindings.

changing this behavior will have knock on effects on other parts of the codebase

Yes. Rust fn pointers are always non-zero, and creating a value that has a invalid bit representation for its type is UB. We want to avoid UBs in the bindings.

[karroffel]

@toasteater

Yes, of course. If you spared a second reading the comments above, you should see

This seems a bit rude and unfair to me. I interpreted the comment as a kind of summary, I did not get the impression that the discussion was not read.

You're right. @bhgsbatista Sorry that I have misinterpreted your comment earlier.

[bhgsbatista]

I appreciate the apology, thanks guys. Since there are knock on effects and the benefits of this request aren't obvious, I'm closing the issue.