Use (r)find_char instead of (r)find for single characters

[AThousandShips]

This method exists for this dedicated purpose and is faster for these kinds of input

[AThousandShips]

Thank you were looking for those but missed them, will convert them

[Repiteo]

Looks good to me!

[Calinou]

Code looks good to me.

[AThousandShips]

Need to remove a few cases that are in text_server_fb/_adv as they don't support this, forgot that, will do so tomorrow (it doesn't apply to the get_slicec PR I made as that method is exposed to scripting/extensions, but find_char is not)

Will also look at exposing (r)find_char as it's useful for extensions (and get_slicec is exposed)

[Repiteo]

Reaffirming that this looks good

If we do end up exposing (r)find_char, maybe we should change the name to (r)findc instead. That'd be more inline with get_slicec, and *_char is a bit verbose of a suffix in hindsight

[AThousandShips]

That won't work with extensions sadly, but for future steps it'd be good to keep in mind

[Repiteo]

Thanks!

[AThousandShips]

Thank you!

[clayjohn]

Folks, I need to insist that performance PRs need need need performance metrics. We absolutely cannot be merging things like this without testing and without confirming that performance does indeed improve.

While the change here makes sense, we still need to validate that before merging otherwise we are just creating noise for potentially no benefit

[AThousandShips]

So to clarify what you mean is specifically measuring the performance gain in-place? As opposed to the fact that these methods are in fact more performant

But I'd question the idea that using the correct method in the correct place, and ensuring uniformity, aren't also objective benefits

[clayjohn]

So to clarify what you mean is specifically measuring the performance gain in-place? As opposed to the fact that these methods are in fact more performant

Without having tested/profiled, you cannot say whether a given method is indeed more performance than another. Performance improvements are not something you can assert a priori, you can form a hypothesis that this method is faster, but then you actually need to test that hypothesis in the code you are seeking to replace.

To be clear, this code seems like it could be marginally faster than the old code, but we just need to actually show that.

Changes like this create a huge amount of noise in the git repo, they force all contributors to do a full (or nearly full recompile), so they need to be justified.

But I'd question the idea that using the correct method in the correct place, and ensuring uniformity, aren't also objective benefits

They can be, but they aren't valuable enough on their own to justify the costs (noise in git, wasted time in CI, wasted developer time in recompiles, risk of regressions).

At the very least stylistic changes need to be discussed and agreed with people from the core team. The objective benefit comes from having a new style that everyone agrees is better. Switching from one arbitrary style to another that only one person prefers is not an objective benefit.

[AThousandShips]

To be clear, I am aware that's why I asked for clarification not an explanation

Then we'd need some good resources on how to test code in-situ where the assumption that a method which itself is faster is indeed faster in-situ

But I think that's something to discuss elsewhere and from a perspective of resources and review policy

[Ivorforce]

Hi, just dropping in post-mortem to document that single-char find calls are expected to be about 1.24x times as fast as full string finds (for medium length strings, likely to be slightly better for long strings). I tested this as part of #100024 :)