Make add_msg_debug and variants a macro.

[prharvey]

Summary

None

Purpose of change

Parameters to add_msg_debug can be expensive, notably monster::name which shows up on profiles as taking up a few % of execution time.

Describe the solution

Turn add_msg_debug into a macro, which allows parameters to be evaluated only if the debug flag is actually enabled.

Testing

Verified that debug messages were still working as intended.

[github-actions]

Auto-requesting reviews from non-collaborators: @jbytheway

[prharvey]

Since this is perf, can we see the before/after profile?

Since this has small effects everywhere it's hard to demonstrate directly, but I can show it indirectly by looking at monster::name

Before:

After:

Although it isn't the few % compared to master mentioned in the summary, as I first noticed this problem on my local version which has #69574 and #69198 on it (well, not the second one directly, since I'm rewriting that, but you get the idea), and those problems are dominating perf at the moment. Once they are in it is closer to 2% on just this function.

[akrieger]

If trying to demonstrate small-but-broad improvement, a good way to show it is to take several traces of a consistent time period (eg. wait the same 6 hours at a location) before & after to show overall runtime improvement. Note that I've found the profiler to have consistent variation of +-5% so improvements less than that may be lost to noise.

[prharvey]

If trying to demonstrate small-but-broad improvement, a good way to show it is to take several traces of a consistent time period (eg. wait the same 6 hours at a location) before & after to show overall runtime improvement. Note that I've found the profiler to have consistent variation of +-5% so improvements less than that may be lost to noise.

This is actually why I mentioned it was hard to show directly currently. Against current master the performance is dominated by much larger problems so it is hard for this to stand out; right now the debug messages not being in a define is more of a code smell than a true performance issue.

That said, when the other changes I plan to make actually land, this will make a noticeable difference. Or at least a more convincing difference when you can actually see functions disappear from the flame graph. If there is any skepticism about the improvement, I'm fine with waiting to merge this until those other things land as well.

I was originally going to get to this optimization much later anyways, but it was pretty easy to do independently and I was reminded of it by a recent NPC AI PR that was adding these extensively.

[akrieger]

Ah, ok. Well code looks fine to me and I understand the reasoning for it.

[akrieger]

I think it'll also seriously matter for #69623, I'm going to patch that and wait at refugee center and see how much it hurts.

[kevingranade]

If there is any skepticism about the improvement, I'm fine with waiting to merge this until those other things land as well.
I'm very much in the wait and see camp here, yes it's potentially needless overhead, but also it's overhead that is going to happen anyway if we are running with debug messages on, and macros are gross. (This is not a criticism of you, I'm not aware of a better way to make this happen either)

[prharvey]

If there is any skepticism about the improvement, I'm fine with waiting to merge this until those other things land as well. I'm very much in the wait and see camp here, yes it's potentially needless overhead, but also it's overhead that is going to happen anyway if we are running with debug messages on, and macros are gross. (This is not a criticism of you, I'm not aware of a better way to make this happen either)

While I can't solve the issues with macros, the majority of users will be playing the game with debug messages off. So I think it is still important to remove overhead in this case.

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. Please do not bump or comment on this issue unless you are actively working on it. Stale issues, and stale issues that are closed are still considered.