-
Notifications
You must be signed in to change notification settings - Fork 47
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.
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
ENHANCE: Change creating callback objects multiple times in pipe operation. #727
base: develop
Are you sure you want to change the base?
Conversation
4833d79
to
556db6e
Compare
5b3ae0e
to
00e791f
Compare
00e791f
to
1ef9240
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
리뷰 완료
1ef9240
to
df7b7e0
Compare
df7b7e0
to
b205d6b
Compare
src/main/java/net/spy/memcached/collection/CollectionPipedInsert.java
Outdated
Show resolved
Hide resolved
b205d6b
to
24dd127
Compare
07918e6
to
ae2a82f
Compare
@brido4125 현재 커밋이 3개입니다. rebase해주셔야 할 것 같습니다. |
24dd127
to
eb87cef
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
리뷰 완료
pipe 연산의 처리, 활용 방식을 정리할 때까지 본 PR 처리는 연기해야 할 것 같습니다. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
리뷰 완료
최신 코드 기준으로 rebase 해 주세요.
super(itemCount); | ||
this.opIdx = opIdx; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
key 설정하는 문장 아래로 옮깁시다.
} | ||
|
||
@Override | ||
public void gotStatus(Integer index, int opIdx, OperationStatus status) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
인자 타입이 Integer, int가 혼용 사용되고 있는 데, int 타입으로 통일하면 될 것 같습니다.
참고로, 아래와 같이 구현할 수 있습니다.
public void gotStatus(int opIdx, int elemIdx, OperationStatus status) {
int index = (opIdx * CollectionPipedUpdate.MAX_PIPED_ITEM_COUNT) + elemIdx;
if (status instanceof CollectionOperationStatus) {
rv.addEachResult(index, (CollectionOperationStatus) status);
} else {
rv.addEachResult(index, new CollectionOperationStatus(status));
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아니면, gotStatus()은 index 인자를 받고,
해당되는 index 값은 gotStatus() 호출하는 쪽에서 계산하여 넘기는 것이 나을 것 같습니다.
public void gotStatus(int index, OperationStatus status) {
if (status instanceof CollectionOperationStatus) {
rv.addEachResult(index, (CollectionOperationStatus) status);
} else {
rv.addEachResult(index, new CollectionOperationStatus(status));
}
}
@@ -1778,15 +1777,11 @@ public <T> CollectionFuture<Map<Integer, CollectionOperationStatus>> asyncBopPip | |||
} | |||
|
|||
List<CollectionPipedInsert<T>> insertList = new ArrayList<CollectionPipedInsert<T>>(); | |||
PartitionedMap<Long, T> list = new PartitionedMap<Long, T>(elements, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PartitionedMap 구현은 주어진 map 크기가 주어진 size 보다 작거나 같을 경우, 해당 map을 그대로 리턴하도록 최적화합시다.
insertList.add(new BTreePipedInsert<T>(key, elementMap, attributesForCreate, tc)); | ||
} | ||
for (int i = 0; i < list.size(); i++) { | ||
insertList.add(new BTreePipedInsert<T>(key, list.get(i), i, attributesForCreate, tc)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
인자 순서 검토 바람: list.get(i), i
=> i, list.get(i)
PR 목록을 살펴보니 아직 pipe 관련된 변경사항이 merge 되진 않은 것 같습니다. |
관련 이슈
https://github.com/jam2in/arcus-works/issues/515
작업 내용
Piped 연산 시 op 인스턴스는 여러개 생성된다.
이와 동시에 callback 인스턴스도 여러개 생성된다.
하나의 콜백 인스턴스를 복수개의 op 인스턴스가 공유해서 사용하도록 변경한다.