Comment 3 for bug 1840053

Revision history for this message
Mike Rylander (mrylander) wrote :

Hi John, thanks for working on this.

Do you have timing information on the improvements, and confirmation that the closures are never reporting old information?

I think it's a good idea to move some logging to the function syntax, but several of these changes make me nervous. In particular, there are logging statements that have side effects (incrementing a variable, say) that won't happen unless the logging level is high enough. I'm also not convinced that the cost of closure creation (and the memory increase it would incur) is worth it for trivial log lines that just stitch together short strings, or run sprintf or the like. Info and error level lines are almost always logged, and should probably have a pretty high bar for function-ification, IMO. If an info or error log line is costly (or noisy) it should probably be lowered to debug or otherwise reworked.

As a general rule, I would be in favor of changing logging that requires an expensive function call IFF it doesn't have any side effects. That may mean removing the side effect, which is probably a good idea anyway. It might be worth evaluating the memory and time cost of moving, say, substantially all debug and internal log lines to the function syntax, compared to always building the log string.