[LUCENE-8476] Optimizations in UserDictionary (KoreanAnalyzer)

• Type: Improvement
• Status: Closed
• Priority: Major
• Resolution: Fixed
• Fix Version/s: 7.5, 8.0
• Component/s: modules/analysis
• Labels: optimization patch-available
• Lucene Fields: New
• Assignee: Unassigned
• Reporter: Namgyu Kim
• Created: 02/Sep/18 17:12
• Updated: 28/Aug/22 15:35
• Resolved: 05/Sep/18 09:37

Description

■ Bug fix

1) BufferedReader's close method is not called.  (Wrong check)

// Line 57 method
public static UserDictionary open(Reader reader) throws IOException {

  BufferedReader br = new BufferedReader(reader);
  String line = null;
  List<String> entries = new ArrayList<>();

  // text + optional segmentations
  while ((line = br.readLine()) != null) {
    ...
  }

  if (entries.isEmpty()) {
    return null;
  } else {
    return new UserDictionary(entries);
  }
}

If you look at the code above, there is no close() method for the "br" variable.
As I know, BufferedReader can cause a memory leak if the close method is not called.
So I changed the code below.

// Line 57 method
public static UserDictionary open(Reader reader) throws IOException {

  String line = null;
  List<String> entries = new ArrayList<>();

  // text + optional segmentations
  try (BufferedReader br = new BufferedReader(reader)) {
    while ((line = br.readLine()) != null) {
      ...
    }
  }

  if (entries.isEmpty()) {
    return null;
  } else {
    return new UserDictionary(entries);
  }
}

I solved this problem with "try-with-resources" method available since Java 7.

 

■ Optimizations

1) Change from Collections.sort to List.sort (UserDictionary constructor)

// Line 82 method
private UserDictionary(List<String> entries) throws IOException {
  final CharacterDefinition charDef = CharacterDefinition.getInstance();
  Collections.sort(entries,
      Comparator.comparing(e -> e.split("\\s+")[0]));

  PositiveIntOutputs fstOutput = PositiveIntOutputs.getSingleton();
  ...
}

List.sort in Java 8 is known to be faster than existing Collections.sort. (http://ankitsambyal.blogspot.com/2014/03/difference-between-listsort-and.html) So I changed the code below.

// Line 82 method
private UserDictionary(List<String> entries) throws IOException {
  final CharacterDefinition charDef = CharacterDefinition.getInstance();
  entries.sort(Comparator.comparing(e -> e.split("\\s+")[0]));

  PositiveIntOutputs fstOutput = PositiveIntOutputs.getSingleton();
  ...
}

 

2) Remove unnecessary null check (UserDictionary constructor)

// Line 82 method
private UserDictionary(List<String> entries) throws IOException {
  ...
  String lastToken = null;
  ...
  for (String entry : entries) {
    String[] splits = entry.split("\\s+");
    String token = splits[0];
    if (lastToken != null && token.equals(lastToken)) {
      continue;
    }
    char lastChar = entry.charAt(entry.length()-1);
  ...
}

Looking at this part of the code,

if (lastToken != null && token.equals(lastToken)) {
  continue;
}

A null check for lastToken is unnecessary.
Because the equals method of the String class internally performs a null check.
So I changed the code as below.

// Line 82 method
private UserDictionary(List<String> entries) throws IOException {
  ...
  String lastToken = null;
  ...
  for (String entry : entries) {
    String[] splits = entry.split("\\s+");
    String token = splits[0];
    if (token.equals(lastToken)) {
      continue;
    }
    char lastChar = entry.charAt(entry.length()-1);
  ...
}