rmuir commented on a change in pull request #311:
URL: https://github.com/apache/lucene/pull/311#discussion_r713746834
##########
File path:
lucene/analysis/common/src/java/org/apache/lucene/analysis/miscellaneous/WordDelimiterGraphFilterFactory.java
##########
@@ -156,14 +156,16 @@ public TokenFilter create(TokenStream input) {
"Invalid Mapping Rule : [" + rule + "]. Only a single character is
allowed.");
if (rhs == null)
throw new IllegalArgumentException("Invalid Mapping Rule : [" + rule +
"]. Illegal type.");
- typeMap.put(lhs.charAt(0), rhs);
+ char c = lhs.charAt(0);
+ typeMap.put(c, rhs);
+ if (c > maxChar) {
+ maxChar = c;
Review comment:
Similar question above, why are we adding this code complexity?
##########
File path:
lucene/analysis/common/src/java/org/apache/lucene/analysis/miscellaneous/WordDelimiterFilterFactory.java
##########
@@ -169,14 +169,16 @@ public TokenFilter create(TokenStream input) {
"Invalid Mapping Rule : [" + rule + "]. Only a single character is
allowed.");
if (rhs == null)
throw new IllegalArgumentException("Invalid Mapping Rule : [" + rule +
"]. Illegal type.");
- typeMap.put(lhs.charAt(0), rhs);
+ char c = lhs.charAt(0);
+ typeMap.put(c, rhs);
+ if (c > maxChar) {
+ maxChar = c;
+ }
Review comment:
what is all this logic? This is a throw-away map (its just a local var
and GC'd). Also it is bounded by Character so it can't have that many entries
in it. Do we really need need to add this complexity?
##########
File path:
lucene/core/src/java/org/apache/lucene/codecs/perfield/PerFieldPostingsFormat.java
##########
@@ -278,7 +278,8 @@ public void close() throws IOException {
private static class FieldsReader extends FieldsProducer {
- private final Map<String, FieldsProducer> fields = new TreeMap<>();
+ private final Map<String, FieldsProducer> fields = new HashMap<>();
+ private final List<String> fieldNames;
Review comment:
TreeMap is a better choice than a map+list, that's too complicated.
##########
File path:
lucene/core/src/java/org/apache/lucene/codecs/perfield/PerFieldDocValuesFormat.java
##########
@@ -262,9 +261,9 @@ static String getFullSegmentSuffix(String
outerSegmentSuffix, String segmentSuff
}
}
- private class FieldsReader extends DocValuesProducer {
+ private static class FieldsReader extends DocValuesProducer {
- private final Map<String, DocValuesProducer> fields = new TreeMap<>();
+ private final Map<String, DocValuesProducer> fields = new HashMap<>();
Review comment:
See also the other PR, with the default codec this is going to increase
memory usage for people with tons of DV fields
--
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-unsubscr...@lucene.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
