Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OAK-11360: remove usage of Guava Ints.contains() #1958

Merged
merged 2 commits into from
Jan 8, 2025
Merged

OAK-11360: remove usage of Guava Ints.contains() #1958

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3f7ebf4
OAK-11360: remove usage of Guava Ints.contains()
reschke Jan 8, 2025
6a5261d
OAK-11360: remove usage of Guava Ints.contains()
reschke Jan 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 5 additions & 8 deletions oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/FieldFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,10 @@

import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Calendar;
import java.util.Collection;
import java.util.Set;

import org.apache.jackrabbit.guava.common.primitives.Ints;
import org.apache.jackrabbit.oak.api.Blob;
import org.apache.jackrabbit.oak.api.Type;
import org.apache.jackrabbit.oak.commons.PathUtils;
Expand All @@ -49,12 +48,12 @@ public final class FieldFactory {

private static final FieldType OAK_TYPE_NOT_STORED = new FieldType();

private static final int[] TYPABLE_TAGS = {
private static final Set<Integer> TYPABLE_TAGS = Set.of(
Type.DATE.tag(),
Type.BOOLEAN.tag(),
Type.DOUBLE.tag(),
Type.LONG.tag(),
};
Type.LONG.tag()
);

static {
OAK_TYPE.setIndexed(true);
Expand All @@ -70,12 +69,10 @@ public final class FieldFactory {
OAK_TYPE_NOT_STORED.setIndexOptions(DOCS_AND_FREQS_AND_POSITIONS);
OAK_TYPE_NOT_STORED.setTokenized(true);
OAK_TYPE_NOT_STORED.freeze();

Arrays.sort(TYPABLE_TAGS);
}

public static boolean canCreateTypedField(Type<?> type) {
return Ints.contains(TYPABLE_TAGS, type.tag());
return TYPABLE_TAGS.contains(type.tag());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return TYPABLE_TAGS.contains(type.tag());
return Arrays.stream(TYPABLE_TAGS).anyMatch(tag -> tag == type.tag());

and we keep the type of TYPABLE_TAGS as int[].

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or we could change TYPABLE_TAGS to Set and take advantage of Set.contains API.

@reschke wdyt ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to avoid to convert ints to Integers every time we do the check.

I also assumed that the original author believed that sorting the array/list would help improve performance.

If this is not the case we of course could make it a Set.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIK List.contains always run in linear time irrespective of whether List is sorted or not.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, sorting only helps if the lower values need to be looked up more frequently.

We could check the code what is really needed here. Or we can just keep things the way they are :-)

}

private final static class OakTextField extends Field {
Expand Down
Loading