You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by "doom369 (via GitHub)" <gi...@apache.org> on 2025/07/08 10:56:29 UTC
[PR] Avoid allocations in IndexFileNames.parseGeneration method [lucene]
doom369 opened a new pull request, #14920:
URL: https://github.com/apache/lucene/pull/14920
`IndexFileNames.parseGeneration` allocates multiple unnecessary strings (3-6 strings per call) + string[] objects:
With often reindexing this allocations become notable during profiling. These allocations could be easily eliminated with a manual underscore search. After the fix only 1 string will be allocated in the worst case.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Avoid allocations in IndexFileNames.parseGeneration method [lucene]
Posted by "doom369 (via GitHub)" <gi...@apache.org>.
doom369 commented on code in PR #14920:
URL: https://github.com/apache/lucene/pull/14920#discussion_r2192873292
##########
lucene/core/src/java/org/apache/lucene/index/IndexFileNames.java:
##########
@@ -145,14 +145,28 @@ public static String stripSegmentName(String filename) {
/** Returns the generation from this file name, or 0 if there is no generation. */
public static long parseGeneration(String filename) {
assert filename.startsWith("_");
- String[] parts = stripExtension(filename).substring(1).split("_");
+ int dot = filename.indexOf('.');
+ int end = (dot != -1) ? dot : filename.length();
+ int start = 1; // skip initial '_'
+
+ int first = filename.indexOf('_', start);
+ if (first == -1 || first >= end) {
+ return 0;
+ }
+
+ int second = filename.indexOf('_', first + 1);
+ int third = (second != -1) ? filename.indexOf('_', second + 1) : -1;
+
+ int parts = (second == -1 || second >= end) ? 2 : (third == -1 || third >= end) ? 3 : 4;
+
Review Comment:
In our scenario, index updates (and we have ~6 different indexes) can happen every 5 seconds. It's profiling info from a 5-minute production run with a async-profiler (one of the best at the marker at the moment, imho). So, if these allocations were captured, they're not rare. Agree about complexity. But this method now should be also 5-10x faster if that matters for you. If you want, I can provide you with a microbenchmark.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Avoid allocations in IndexFileNames.parseGeneration method [lucene]
Posted by "doom369 (via GitHub)" <gi...@apache.org>.
doom369 commented on code in PR #14920:
URL: https://github.com/apache/lucene/pull/14920#discussion_r2192873292
##########
lucene/core/src/java/org/apache/lucene/index/IndexFileNames.java:
##########
@@ -145,14 +145,28 @@ public static String stripSegmentName(String filename) {
/** Returns the generation from this file name, or 0 if there is no generation. */
public static long parseGeneration(String filename) {
assert filename.startsWith("_");
- String[] parts = stripExtension(filename).substring(1).split("_");
+ int dot = filename.indexOf('.');
+ int end = (dot != -1) ? dot : filename.length();
+ int start = 1; // skip initial '_'
+
+ int first = filename.indexOf('_', start);
+ if (first == -1 || first >= end) {
+ return 0;
+ }
+
+ int second = filename.indexOf('_', first + 1);
+ int third = (second != -1) ? filename.indexOf('_', second + 1) : -1;
+
+ int parts = (second == -1 || second >= end) ? 2 : (third == -1 || third >= end) ? 3 : 4;
+
Review Comment:
In our scenario, index updates (and we have ~6 different indexes) can happen every 5 seconds. It's profiling info from a 5-minute production run. So, if these allocations were captured, they're not rare. Agree about complexity. But this method now should be also 5-10x faster if that matters for you. If you want, I can provide you with a microbenchmark.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Avoid allocations in IndexFileNames.parseGeneration method [lucene]
Posted by "doom369 (via GitHub)" <gi...@apache.org>.
doom369 closed pull request #14920: Avoid allocations in IndexFileNames.parseGeneration method
URL: https://github.com/apache/lucene/pull/14920
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Avoid allocations in IndexFileNames.parseGeneration method [lucene]
Posted by "doom369 (via GitHub)" <gi...@apache.org>.
doom369 commented on code in PR #14920:
URL: https://github.com/apache/lucene/pull/14920#discussion_r2192873292
##########
lucene/core/src/java/org/apache/lucene/index/IndexFileNames.java:
##########
@@ -145,14 +145,28 @@ public static String stripSegmentName(String filename) {
/** Returns the generation from this file name, or 0 if there is no generation. */
public static long parseGeneration(String filename) {
assert filename.startsWith("_");
- String[] parts = stripExtension(filename).substring(1).split("_");
+ int dot = filename.indexOf('.');
+ int end = (dot != -1) ? dot : filename.length();
+ int start = 1; // skip initial '_'
+
+ int first = filename.indexOf('_', start);
+ if (first == -1 || first >= end) {
+ return 0;
+ }
+
+ int second = filename.indexOf('_', first + 1);
+ int third = (second != -1) ? filename.indexOf('_', second + 1) : -1;
+
+ int parts = (second == -1 || second >= end) ? 2 : (third == -1 || third >= end) ? 3 : 4;
+
Review Comment:
In our scenario, index updates (and we have ~6 different indexes) can happen every 5 seconds. It's profiling info from a 5-minute production run with a async-profiler (one of the best at the moment, imho). So, if these allocations were captured, they're not rare. Agree about complexity. But this method now should be also 5-10x faster if that matters for you. If you want, I can provide you with a microbenchmark.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Avoid allocations in IndexFileNames.parseGeneration method [lucene]
Posted by "rmuir (via GitHub)" <gi...@apache.org>.
rmuir commented on PR #14920:
URL: https://github.com/apache/lucene/pull/14920#issuecomment-3048549026
> With often reindexing this allocations become notable during profiling.
Your profiler is lying to you. Toss it and get a better one :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Avoid allocations in IndexFileNames.parseGeneration method [lucene]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #14920:
URL: https://github.com/apache/lucene/pull/14920#issuecomment-3408720395
This PR has not had activity in the past 2 weeks, labeling it as stale. If the PR is waiting for review, notify the dev@lucene.apache.org list. Thank you for your contribution!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Avoid allocations in IndexFileNames.parseGeneration method [lucene]
Posted by "doom369 (via GitHub)" <gi...@apache.org>.
doom369 commented on code in PR #14920:
URL: https://github.com/apache/lucene/pull/14920#discussion_r2192873292
##########
lucene/core/src/java/org/apache/lucene/index/IndexFileNames.java:
##########
@@ -145,14 +145,28 @@ public static String stripSegmentName(String filename) {
/** Returns the generation from this file name, or 0 if there is no generation. */
public static long parseGeneration(String filename) {
assert filename.startsWith("_");
- String[] parts = stripExtension(filename).substring(1).split("_");
+ int dot = filename.indexOf('.');
+ int end = (dot != -1) ? dot : filename.length();
+ int start = 1; // skip initial '_'
+
+ int first = filename.indexOf('_', start);
+ if (first == -1 || first >= end) {
+ return 0;
+ }
+
+ int second = filename.indexOf('_', first + 1);
+ int third = (second != -1) ? filename.indexOf('_', second + 1) : -1;
+
+ int parts = (second == -1 || second >= end) ? 2 : (third == -1 || third >= end) ? 3 : 4;
+
Review Comment:
In our scenario, index updates can happen every 5 seconds. It's profiling info from a 5-minute production run. So, if these allocations were captured, they're not rare. Agree about complexity. But this method now should be also 5-10x faster if that matters for you. If you want, I can provide you with a microbenchmark.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Avoid allocations in IndexFileNames.parseGeneration method [lucene]
Posted by "rmuir (via GitHub)" <gi...@apache.org>.
rmuir commented on code in PR #14920:
URL: https://github.com/apache/lucene/pull/14920#discussion_r2192263267
##########
lucene/core/src/java/org/apache/lucene/index/IndexFileNames.java:
##########
@@ -145,14 +145,28 @@ public static String stripSegmentName(String filename) {
/** Returns the generation from this file name, or 0 if there is no generation. */
public static long parseGeneration(String filename) {
assert filename.startsWith("_");
- String[] parts = stripExtension(filename).substring(1).split("_");
+ int dot = filename.indexOf('.');
+ int end = (dot != -1) ? dot : filename.length();
+ int start = 1; // skip initial '_'
+
+ int first = filename.indexOf('_', start);
+ if (first == -1 || first >= end) {
+ return 0;
+ }
+
+ int second = filename.indexOf('_', first + 1);
+ int third = (second != -1) ? filename.indexOf('_', second + 1) : -1;
+
+ int parts = (second == -1 || second >= end) ? 2 : (third == -1 || third >= end) ? 3 : 4;
+
Review Comment:
This is a massive increase in complexity of the code: the tradeoff isn't worth it.
It's a simple string method called once, doesn't even approach the cost of the system call to actually read the file.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Avoid allocations in IndexFileNames.parseGeneration method [lucene]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #14920:
URL: https://github.com/apache/lucene/pull/14920#issuecomment-3048410322
This PR does not have an entry in lucene/CHANGES.txt. Consider adding one. If the PR doesn't need a changelog entry, then add the skip-changelog label to it and you will stop receiving this reminder on future updates to the PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Avoid allocations in IndexFileNames.parseGeneration method [lucene]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #14920:
URL: https://github.com/apache/lucene/pull/14920#issuecomment-3105235358
This PR has not had activity in the past 2 weeks, labeling it as stale. If the PR is waiting for review, notify the dev@lucene.apache.org list. Thank you for your contribution!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org