+1. To not using streams in hot paths. Regarding string concatenation in logging, for debug and trace it makes sense to avoid concatenation. For info and error I don't think it matters and it can be more concise to concatenate. It's not a big deal to standardize on one just because the extra verbosity is not that bad.
Ariel On Thu, May 30, 2024, at 12:29 PM, Benedict wrote: > > Since it’s related to the logging discussion we’re already having, I have > seen stream pipelines showing up in a lot of traces recently. I am surprised; > I thought it was understood that they shouldn’t be used on hot paths as they > are not typically as efficient as old skool for-each constructions done > sensibly, especially for small collections that may normally take zero or one > items. > > I would like to propose forbidding the use of streams on hot paths without > good justification that the cost:benefit is justified. > > It looks like it was nominally agreed two years ago that we would include > words to this effect in the code style guide, but I forgot to include them > when I transferred the new contents from the Google Doc proposal. So we could > just include the “Performance” section that was meant to be included at the > time. > > lists.apache.org > <https://lists.apache.org/thread/1mt8rsg36p1mq8s8578l6js075lrmvlt> > favicon.ico <https://lists.apache.org/thread/1mt8rsg36p1mq8s8578l6js075lrmvlt> > > > >> On 30 May 2024, at 13:33, Štefan Miklošovič <stefan.mikloso...@gmail.com> >> wrote: >> >> I see the feedback is overall positive. I will merge that and I will improve >> the documentation on the website along with what Benedict suggested. >> >> On Thu, May 30, 2024 at 10:32 AM Mick Semb Wever <m...@apache.org> wrote: >>> >>> >>>> Based on these findings, I went through the code and I have incorporated >>>> these rules and I rewrote it like this: >>>> >>>> 1) no wrapping in "if" if we are not logging more than 2 parameters. >>>> 2) rewritten log messages to not contain any string concatenation but >>>> moving it all to placeholders ({}). >>>> 3) wrap it in "if" if we need to execute a method(s) on parameter(s) which >>>> is resource-consuming. >>> >>> >>> +1 >>> >>> >>> It's a shame slf4j botched it with lambdas, their 2.0 fluent api doesn't >>> impress me.
