Save regexp Pattern allocation on config name renaming #39444
Conversation
quarkus-bot
bot
added
area/core
area/oidc
area/reactive-messaging
area/rest
area/smallrye
labels
| if (!name.startsWith(oldPrefix)) { | ||
| return name; | ||
| } | ||
| // don't use here String::replaceFirst because it uses regex |
There was a problem hiding this comment.
Ah f**k. I thought replaceFirst was like replace and not using regexp :/
There was a problem hiding this comment.
Yeah, I was surprised as well! Said that, you know that this change I'm pushing is a drop in the ocean which value is mostly on benchs as far as I can tell :/
And likely, the renaming will goes away in few releases too
gsmet
force-pushed
the
no_pattern_allocation
branch
from
fa48b97 to
73ff652
Compare
|
|
||
| @Test | ||
| public void testChangePrefix() { | ||
| assertEquals("quarkus.new-prefix.configuration-property", StringUtil |
| private static final Function<String, String> RENAME_FUNCTION = new Function<String, String>() { | ||
| @Override | ||
| public String apply(String s) { | ||
| return StringUtil.changePrefix(s, NEW_PREFIX, OLD_PREFIX); |
There was a problem hiding this comment.
@gsmet My ide has been smarter than me and I fix it back (making it wrong) later on, uff :/ many thanks to have found it!
|
Thanks @gsmet for the review 🙏 |
Status for workflow
|
|
Thanks, good catch! |
This is addressing a small regression in case some old names are used, saving allocating (for each replacement) a whole new regexp
Patternand creating a reusable function which does it "right" if compared toString:.replaceFirst.I've than removed the method reference lambda generation to match what we do elsewhere, but I have not a strong opinion here, considered that it mostly affect RSS footprint while the application doesn't do anything interesting (sadly a case still valuable in public benchmarks :/ ) - see #36204 (comment) for more info at
java/lang/invoke/LambdaForm.compileToBytecode.In JDK 21 it's likely lambdas RSS footprint is reduced and who knows for later releases, so, as said, I've performed the change here, but I could have made the interceptors to implements
Function<String, Stringand just passthisto theirsuperconstructor too, not getting lambdas nor anonymous class loaded, at all.