The Great Debuggening

[ocornoc]

Every type in Bevy now has Debug implemented.

Some caveats:

• A few dependencies' types (namely rodio::Device, notify::RecommendedWatcher, and guillotiere::AtlasAllocator) don't implement Debug, so I got around this by having custom empty structs in the relevant Debug impls that have the message Type doesn't implement Debug for that field value.
• dyns are turned into pointers so that they can be formatted. This could also be solved by making Debug be a supertrait of all Resources etc, but that's easy to change to and adds extra constraints, so I didn't do this in this PR. I'm happy to change it to be that though. With this, at least the pointers are visible and the rest of the type is formatted correctly

[cart]

Id like to collect a couple of datapoints on how compile times are affected by this. Can you clean-compile the base of this branch, record the time, then clean compile with these changes? And also do the same for iterative compiles? Ex: compile and run 3d_scene, then insert a newline into 3d_scene, then recompile and record the time?

[cart]

I'll do the same before merging.

[ocornoc]

Sure. I'll do it both on my Windows desktop and my chromebook. I'm just gonna finish satiating clippy first, so give me at most an hour.

[ocornoc]

Apparently on my Windows desktop, I get a linker error code 1120 with the note:

          libbevy_type_registry-f4c70392ccd84e77.rlib(bevy_type_registry-f4c70392ccd84e77.1lqn52xvdw5f8npf.rcgu.o) : error LNK2001: unresolved external symbol _ZN4core3ptr13drop_in_place17hd46a668710b48583E
          libbevy_type_registry-f4c70392ccd84e77.rlib(bevy_type_registry-f4c70392ccd84e77.1lqn52xvdw5f8npf.rcgu.o) : error LNK2001: unresolved external symbol _ZN4core3ptr13drop_in_place17he454ff19ea980ff8E
          libbevy_type_registry-f4c70392ccd84e77.rlib(bevy_type_registry-f4c70392ccd84e77.1lqn52xvdw5f8npf.rcgu.o) : error LNK2001: unresolved external symbol _ZN4core3ptr13drop_in_place17hcb1f782b52cd0c0aE
          libbevy_property-ace11071b5d20d37.rlib(bevy_property-ace11071b5d20d37.18pxu7k0gyq0jczo.rcgu.o) : error LNK2001: unresolved external symbol _ZN4core3ptr13drop_in_place17hdfe318eabba20ffdE
          C:\Users\me\Desktop\nstar\bevy\target\debug\examples\3d_scene.exe : fatal error LNK1120: 4 unresolved externals

Exciting! This error doesn't show up on the base of the branch, but does with my commits.

It seems like these are places where I'm converting a function pointer into another with lifetimes that allow Debug to format them using the default implementation.

[ocornoc]

Windows

Base/PR	Kind	Time to debug compile and run 3d_scene	Time run 2
Base	Clean	1m 45s	1m 42s
Base	Incremental	3.61s	3.57s
PR	Clean	1m 25s	1m 27s
PR	Incremental	3.67s	3.69s

I don't know why it got faster, I'll see if I can reproduce it.

Chromebook (Debian Crostini)

Base/PR	Kind	Time to debug compile and run 3d_scene	Time run 2
Base	Clean	3m 58s	4m 12s
Base	Incremental	12.35s	12.92s
PR	Clean	4m 02s	4m 05s
PR	Incremental	12.80s	12.23s

[CleanCut]

In macOS on a 16-core (32 logical) Mac Pro in debug mode (all with 4 warm runs, averaged) - clean builds of the 3d_scene example were within margin of error on master vs this PR: 58.66s vs 58.82s (0.3% slower), as were incremental builds: 10.09s vs 10.04s (0.5% faster).

More interesting to me was the incremental when using bevy as a library from an external project with all the bevy_template optimizations turned on: 0.925s vs 0.935s (1% slower) -- of course, these incremental compiles are also ten times faster with all the optimizations turned on than without the optimizations, which is fun to see. Without them, incremental compiles of bevy_template take ~10.05s or so just like a built-in example. Those optimizations are a huge quality-of-life booster for me.

[ocornoc]

I would've thought that the impact would've been at least noticeable. Rust is impressive.

[cart]

I ran tests and I can confirm that its all in the noise on my machine too. Starting a review now!

[cart]

In general this looks good, but I think theres been a "impl Debug at all costs" mindset here, which I personally don't agree with. For derive(Debug) thats not as big of a deal, but its important to call out that custom Debug impls have a number of negative implications:

• not resilient to field renames
• not resilient to field additions
• add code bloat + maintenance burden

I'd appreciate it if we took a second pass and applied the following criteria, with a general mindset of "remove as many custom debug impls as possible" and "remove Debug derives from things that really don't need them":

Things that don't need custom debug impls:

• structs that just print out a pointer or set of pointers
• iterators
• private implementation details like "intermediate serde helpers" and "fetch types in bevy_ecs". keep in mind that people developing these internal implementation details have the freedom to print the things they want to test / impl what they want.
• "large" structs that produce a lot of noise by printing them, especially when the "useful" debuggable fields are individually accessible.

Things that should have custom debug impls:

• Public-facing components / resources / assets that contain "useful" information, but cant derive debug
  • Make sure the debug impl avoids printing "useless" information. If a field isn't debuggable id rather not use the NoDebug pattern. NoDebug introduces a lot of code bloat and just produces noise in the printed debug info. Id prefer to just not print those fields.

Things that don't need debug derives:

• unit structs
• types where you can't imagine a reasonable scenario where someone would want the info printed

[ocornoc]

Alright, I'll do another pass today. Thanks!

[cart]

Awesome. Thanks for putting in the work here. People are really going to appreciate this :)

[cart]

Also none of the criteria I listed are hard rules. Feel free to use your own judgement on a case-by-case basis and we can talk about cases where we don't agree.