INTERNAL: Add new merge method with improved performance to old SMGetResult.

[uhm0311]

https://github.com/jam2in/arcus-works/issues/482

SMGetResultOldImpl 클래스에 성능이 개선된 merge 메소드를 추가했습니다.
git diff 결과에 기존 코드와 새 코드가 섞이지 않도록 메소드를 추가하는 방식으로 작업했습니다.

기존 코드 : 기존에 갖고 있던 결과에 입력된 결과의 요소들을 중간에 끼워 넣는 방식입니다.
개선된 코드 : 기존에 갖고 있던 결과와 입력된 결과를 새로운 결과에 하나씩 넣는 방식입니다.

[jhpark816]

@brido4125 @oliviarla 리뷰 바랍니다.

[brido4125]

성능 향상된 메서드와 기존 메서드를 별도로 유지하기로 결정이 난 사항인가요?
리뷰를 위해 따로 분리시켜놓으신건가요?

[brido4125]

꼭 do ... while(false) 형태의 구문을 사용해야 하나요?
단순히 생각햇을 때 do 내부 로직을 한번 실행시킨다라고 이해하였는데
다른 의미가 있을까요?

[uhm0311]

리뷰를 위해 따로 분리시켜놓으신건가요?

첫번째 코멘트에 git diff 결과에 기존 코드와 새 코드가 섞이지 않도록 메소드를 추가하는 방식으로 작업했습니다. 라고 명시했습니다.

꼭 do ... while(false) 형태의 구문을 사용해야 하나요?

do while (false) 구문은 로직을 중간에 건너뛰고 싶을 때 사용할 수 있습니다.
일반적으로 알고 있는 방법 중에서 중간 로직을 수행하지 않으려면 void 메소드에서 return문을 사용하면 되는데, 이러면 메소드에서 빠져나오게 됩니다.
그래서 현재 구현과 같이 어떤 조건 하에서 중간 로직을 건너뛰고 마지막 공통 로직을 항상 수행하고 싶다면 do while (false) 구문을 사용하는 블록 전체를 별도의 메소드로 만들어야 합니다.
새로운 Merge 메소드에서 항상 수행해야 하는 마지막 공통 로직은 mergedResult = newMergedResult;가 있겠죠.
do while (false) 구문을 사용한 것은 다음과 같이 2개의 메소드로 분리된 로직을 1개의 메소드 내에 넣을 때 사용 가능합니다.

  public void mergeSMGetElements2(final List<SMGetElement<T>> eachResult,
                                  final boolean isEachResultTrimmed) {

    if (mergedResult.isEmpty()) {
      // merged result is empty, add all.
      mergedResult.addAll(eachResult);
      isMergedResultTrimmed.set(isEachResultTrimmed);

      while (mergedResult.size() > totalResultElementCount) {
        mergedResult.remove(totalResultElementCount);
      }
      return;
    }

    final int eachSize = eachResult.size();
    final int mergedSize = mergedResult.size();
    final List<SMGetElement<T>> newMergedResult
            = new ArrayList<SMGetElement<T>>(totalResultElementCount);

    int eachPos = 0, mergedPos = 0, comp = 0;
    boolean allAdded = false; // Is all element of eachResult added to mergedResult?

    while (eachPos < eachSize && mergedPos < mergedSize
            && newMergedResult.size() < totalResultElementCount) {

      final SMGetElement<T> each = eachResult.get(eachPos);
      final SMGetElement<T> merged = mergedResult.get(mergedPos);

      comp = each.compareTo(merged);
      if ((reverse) ? (comp > 0) : (comp < 0)) {
        newMergedResult.add(each);
        eachPos++;

        if (eachPos >= eachSize) {
          allAdded = true;

          if (isEachResultTrimmed) {
            // Can NOT add to the trimmed area of eachResult.
            isMergedResultTrimmed.set(true);
            break;
          }
        }
      } else if ((reverse) ? (comp < 0) : (comp > 0)) {
        newMergedResult.add(merged);
        mergedPos++;
      } else {
        // comp == 0
        mergedPos++;
      }
    }

    temp(eachResult, isEachResultTrimmed, newMergedResult,
            mergedPos, mergedSize, eachPos, eachSize, comp, allAdded);

    if (newMergedResult.size() >= totalResultElementCount) {
      // If size of mergedResult is reached to totalResultElementCount,
      // then mergedResult is NOT trimmed.
      isMergedResultTrimmed.set(false);
    }

    mergedResult = newMergedResult;
  }
  
  public void temp(List<SMGetElement<T>> eachResult, 
                   boolean isEachResultTrimmed,
                   List<SMGetElement<T>> newMergedResult,
                   int mergedPos, 
                   int mergedSize, 
                   int eachPos, 
                   int eachSize,
                   int comp, 
                   boolean allAdded) {
    
    if (mergedPos >= mergedSize && isMergedResultTrimmed.get() && comp != 0) {
      // Can NOT add to the trimmed area of mergedResult.
      return;
    }
    while (eachPos < eachSize && newMergedResult.size() < totalResultElementCount) {
      newMergedResult.add(eachResult.get(eachPos++));

      if (eachPos >= eachSize) {
        allAdded = true;
      }
    }
    if (isEachResultTrimmed && allAdded) {
      // Can NOT add to the trimmed area of eachResult.
      isMergedResultTrimmed.set(true);
      return;
    }
    while (mergedPos < mergedSize && newMergedResult.size() < totalResultElementCount) {
      newMergedResult.add(mergedResult.get(mergedPos++));
    }
  }

[oliviarla]

리뷰 완료입니다.

[oliviarla]

당장 반영되진 않을 사항이겠지만 개인적으로 reverse라는 의미가 바로 와닿지 않아서 bkeyDesc/bkeyAsc 와 같이 순서를 정확히 나타내는 용어를 사용하면 좋을 것 같습니다.

[oliviarla]

이 경우는 동일한 bkey가 새로운 result와 기존 mergedResult에 존재하는 경우인 것 같은데, mergedResult의 element는 newMergedResult에 추가되지 않고 무시되는 걸까요?

[uhm0311]

캐시 키와 bkey가 모두 동일한 경우입니다.
실제 연산에선 해당 사항이 없으나, 여러 경우를 테스트 하던 중 데이터 셋을 잘못 만들어서 캐시 키와 bkey가 동일한 경우가 있었는데 해당 경우에 대한 처리를 하지 않으면 무한 루프에 빠지게 되어 추가하게 되었습니다.
캐시 키와 bkey가 완전히 동일한 Element라면 eachResult에서 가져오든, mergedResult에서 가져오든 상관은 없지만 기존 Merge 메소드의 구현이 캐시 키와 bkey가 동일한 경우에 mergedResult를 무시하는 것으로 되어 있어서 똑같이 구현했습니다.

[jhpark816]

@uhm0311
old smget은 deprecated 상태라,
new smget 구현에 대해 PR 보내주는 것이 나을 것 같습니다.

[jhpark816]

@uhm0311 본 PR은 close 바랍니다.

[uhm0311]

@jhpark816

#711 (comment)
기존 API를 Deprecated 처리한다고 해서 모든 응용이 Old smget을 사용하지 않는 것은 아닙니다.
본 PR을 적용하지 않으면 Old smget은 여전히 개선된 방안에 비해서는 IO Thread에 작업 부담을 더 주는 형태로 있게 되는데요.
다음 릴리즈 버전에서 Old smget API 자체를 제거하기 위해 Close를 하는 것인가요?

[jhpark816]

@uhm0311
old smget은 현재 권고하지 않는 기능이므로, 새로운 응용에서 old smget 사용하는 경우는 없을 것입니다.
기존 응용에서 old smget 사용하고 있지만, 기존 old smget 코드 기반으로 현재 잘 돌아가고 있는 상태입니다.
따라서, 이를 최적화하기 위해 기존 코드를 수정할 이유가 없는 것 같습니다.