You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@iotdb.apache.org by GitBox <gi...@apache.org> on 2021/09/07 13:47:37 UTC
[GitHub] [iotdb] DittoTool opened a new pull request #3926: Inefficient Usage of JCF
DittoTool opened a new pull request #3926:
URL: https://github.com/apache/iotdb/pull/3926
Hi,
We find that there are several inefficient usages of Java Collections:
1. The contains method is invoked upon a list object in a loop. We recommend replacing it with a HashSet.
2. Random access can occur at several linkedlist objects, which run in linear time complexity. We recommend replacing them with Arraylist objects.
3. There is no iteration occurring upon a LinkedHashMap, thus the insertion order does not matter. We recommend replacing it with a HashMap.
4. ArrayList is inserted before an iteration, while multiple memory reallocation might occur when the size of the list exceeds its capacity. We recommend replacing it with a LinkedList.
We discovered the above inefficient usage of containers by our tool Ditto. The patch is submitted. Could you please check and accept it? We have tested the patch on our PC. The patched program works well.
Bests
Ditto
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] HTHou commented on a change in pull request #3926: [ISSUE-3927] Inefficient Usage of JCF
Posted by GitBox <gi...@apache.org>.
HTHou commented on a change in pull request #3926:
URL: https://github.com/apache/iotdb/pull/3926#discussion_r709149119
##########
File path: server/src/main/java/org/apache/iotdb/db/query/executor/fill/LastPointReader.java
##########
@@ -169,8 +170,9 @@ private TimeValuePair getChunkLastPoint(IChunkMetadata chunkMetaData) throws IOE
chunkStatistics.getEndTime(), chunkStatistics.getLastValue(), dataType);
}
List<IPageReader> pageReaders = FileLoaderUtils.loadPageReaderList(chunkMetaData, timeFilter);
- for (int i = pageReaders.size() - 1; i >= 0; i--) {
- IPageReader pageReader = pageReaders.get(i);
+ Iterator it = pageReaders.descendingIterator();
Review comment:
```suggestion
Iterator it = ((LinkedList<IPageReader>) pageReaders).descendingIterator();
```
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] SteveYurongSu commented on pull request #3926: [ISSUE-3927] Inefficient Usage of JCF
Posted by GitBox <gi...@apache.org>.
SteveYurongSu commented on pull request #3926:
URL: https://github.com/apache/iotdb/pull/3926#issuecomment-915041849
#3927
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] coveralls edited a comment on pull request #3926: [ISSUE-3927] Inefficient Usage of JCF
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3926:
URL: https://github.com/apache/iotdb/pull/3926#issuecomment-914365215
[](https://coveralls.io/builds/42764558)
Coverage decreased (-0.01%) to 67.41% when pulling **380af61a6c97615729ed60387a0a80197048f99f on DittoTool:master** into **aaa96b974becf7cc57ef5ac36c3fa9e199423010 on apache:master**.
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] HTHou commented on a change in pull request #3926: [ISSUE-3927] Inefficient Usage of JCF
Posted by GitBox <gi...@apache.org>.
HTHou commented on a change in pull request #3926:
URL: https://github.com/apache/iotdb/pull/3926#discussion_r709152919
##########
File path: server/src/main/java/org/apache/iotdb/db/query/executor/fill/LastPointReader.java
##########
@@ -35,6 +35,7 @@
import java.io.IOException;
import java.util.ArrayList;
+import java.util.Iterator;
Review comment:
```suggestion
import java.util.Iterator;
import java.util.LinkedList;
```
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] SteveYurongSu commented on a change in pull request #3926: Inefficient Usage of JCF
Posted by GitBox <gi...@apache.org>.
SteveYurongSu commented on a change in pull request #3926:
URL: https://github.com/apache/iotdb/pull/3926#discussion_r704133554
##########
File path: server/src/main/java/org/apache/iotdb/db/engine/storagegroup/TsFileProcessor.java
##########
@@ -746,7 +747,8 @@ public void syncFlush() throws IOException {
synchronized (tmpMemTable) {
try {
long startWait = System.currentTimeMillis();
- while (flushingMemTables.contains(tmpMemTable)) {
+ HashSet<IMemTable> flushingMemSet = new HashSet<>(flushingMemTables);
+ while (flushingMemSet.contains(tmpMemTable)) {
Review comment:
I think the change here may cause concurrent issues: `flushingMemTables` may be changed by other working threads.
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] coveralls edited a comment on pull request #3926: [ISSUE-3927] Inefficient Usage of JCF
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3926:
URL: https://github.com/apache/iotdb/pull/3926#issuecomment-914365215
[](https://coveralls.io/builds/42721715)
Coverage decreased (-0.02%) to 67.406% when pulling **8804ab08f4b637307a34f5c42236a4087e2bc486 on DittoTool:master** into **aaa96b974becf7cc57ef5ac36c3fa9e199423010 on apache:master**.
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] DittoTool commented on a change in pull request #3926: Inefficient Usage of JCF
Posted by GitBox <gi...@apache.org>.
DittoTool commented on a change in pull request #3926:
URL: https://github.com/apache/iotdb/pull/3926#discussion_r704001836
##########
File path: tsfile/src/main/java/org/apache/iotdb/tsfile/read/reader/chunk/VectorChunkReader.java
##########
@@ -59,7 +59,7 @@
protected Filter filter;
- private final List<IPageReader> pageReaderList = new LinkedList<>();
+ private final List<IPageReader> pageReaderList = new ArrayList<>();
Review comment:
Similarly, this list object is also randomly accessed in a loop
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] HTHou commented on a change in pull request #3926: [ISSUE-3927] Inefficient Usage of JCF
Posted by GitBox <gi...@apache.org>.
HTHou commented on a change in pull request #3926:
URL: https://github.com/apache/iotdb/pull/3926#discussion_r709149119
##########
File path: server/src/main/java/org/apache/iotdb/db/query/executor/fill/LastPointReader.java
##########
@@ -169,8 +170,9 @@ private TimeValuePair getChunkLastPoint(IChunkMetadata chunkMetaData) throws IOE
chunkStatistics.getEndTime(), chunkStatistics.getLastValue(), dataType);
}
List<IPageReader> pageReaders = FileLoaderUtils.loadPageReaderList(chunkMetaData, timeFilter);
- for (int i = pageReaders.size() - 1; i >= 0; i--) {
- IPageReader pageReader = pageReaders.get(i);
+ Iterator it = pageReaders.descendingIterator();
Review comment:
```suggestion
Iterator it = ((LinkedList<IPageReader>)pageReaders).descendingIterator();
```
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] HTHou commented on a change in pull request #3926: [ISSUE-3927] Inefficient Usage of JCF
Posted by GitBox <gi...@apache.org>.
HTHou commented on a change in pull request #3926:
URL: https://github.com/apache/iotdb/pull/3926#discussion_r711518575
##########
File path: server/src/main/java/org/apache/iotdb/db/query/executor/fill/LastPointReader.java
##########
@@ -169,8 +171,9 @@ private TimeValuePair getChunkLastPoint(IChunkMetadata chunkMetaData) throws IOE
chunkStatistics.getEndTime(), chunkStatistics.getLastValue(), dataType);
}
List<IPageReader> pageReaders = FileLoaderUtils.loadPageReaderList(chunkMetaData, timeFilter);
- for (int i = pageReaders.size() - 1; i >= 0; i--) {
- IPageReader pageReader = pageReaders.get(i);
+ Iterator it = ((LinkedList<IPageReader>) pageReaders).descendingIterator();
Review comment:
```suggestion
if (pageReaders instanceof LinkedList) {
Iterator it = ((LinkedList<IPageReader>) pageReaders).descendingIterator();
} else {
// for singleton list
Iterator it = pageReaders.iterator();
}
```
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] Alima777 commented on a change in pull request #3926: Inefficient Usage of JCF
Posted by GitBox <gi...@apache.org>.
Alima777 commented on a change in pull request #3926:
URL: https://github.com/apache/iotdb/pull/3926#discussion_r704016084
##########
File path: tsfile/src/main/java/org/apache/iotdb/tsfile/read/reader/chunk/ChunkReader.java
##########
@@ -55,7 +55,7 @@
protected Filter filter;
- private List<IPageReader> pageReaderList = new LinkedList<>();
+ private List<IPageReader> pageReaderList = new ArrayList<>();
Review comment:
Hi, I didn't notice that. If that, maybe use a iterator here to traverse the list is better. In this way we can do these two methods both efficiently.
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] coveralls edited a comment on pull request #3926: Inefficient Usage of JCF
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3926:
URL: https://github.com/apache/iotdb/pull/3926#issuecomment-914365215
[](https://coveralls.io/builds/42720811)
Coverage increased (+0.1%) to 67.519% when pulling **204c7ffc8ea39c17a041fcb2f893253e39a887c1 on DittoTool:master** into **aaa96b974becf7cc57ef5ac36c3fa9e199423010 on apache:master**.
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] coveralls edited a comment on pull request #3926: [ISSUE-3927] Inefficient Usage of JCF
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3926:
URL: https://github.com/apache/iotdb/pull/3926#issuecomment-914365215
[](https://coveralls.io/builds/42722465)
Coverage increased (+0.1%) to 67.534% when pulling **a1f89ebe647c5c2c26c95185ea1504776feb39e1 on DittoTool:master** into **aaa96b974becf7cc57ef5ac36c3fa9e199423010 on apache:master**.
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] DittoTool commented on a change in pull request #3926: Inefficient Usage of JCF
Posted by GitBox <gi...@apache.org>.
DittoTool commented on a change in pull request #3926:
URL: https://github.com/apache/iotdb/pull/3926#discussion_r704001611
##########
File path: tsfile/src/main/java/org/apache/iotdb/tsfile/read/reader/chunk/ChunkReader.java
##########
@@ -55,7 +55,7 @@
protected Filter filter;
- private List<IPageReader> pageReaderList = new LinkedList<>();
+ private List<IPageReader> pageReaderList = new ArrayList<>();
Review comment:
We think the following part actually performs random access upon the list object frequently in a loop. It would be quite expensive. Compare to the remove(0) you mentioned, I still think it would be better to change it to ArrayList. Remove(0) would invoke the copy of a successive memory, which can be conducted in a relatively efficient way.
https://github.com/apache/iotdb/blob/aaa96b974becf7cc57ef5ac36c3fa9e199423010/server/src/main/java/org/apache/iotdb/db/query/executor/fill/LastPointReader.java#L171-L173
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] coveralls edited a comment on pull request #3926: Inefficient Usage of JCF
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3926:
URL: https://github.com/apache/iotdb/pull/3926#issuecomment-914365215
[](https://coveralls.io/builds/42704832)
Coverage increased (+0.09%) to 67.509% when pulling **2d6fe1154114bed3fe0cc7be1d5eaef30be70088 on DittoTool:master** into **aaa96b974becf7cc57ef5ac36c3fa9e199423010 on apache:master**.
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] DittoTool commented on a change in pull request #3926: Inefficient Usage of JCF
Posted by GitBox <gi...@apache.org>.
DittoTool commented on a change in pull request #3926:
URL: https://github.com/apache/iotdb/pull/3926#discussion_r704019589
##########
File path: tsfile/src/main/java/org/apache/iotdb/tsfile/read/reader/chunk/ChunkReader.java
##########
@@ -55,7 +55,7 @@
protected Filter filter;
- private List<IPageReader> pageReaderList = new LinkedList<>();
+ private List<IPageReader> pageReaderList = new ArrayList<>();
Review comment:
Wow. It is the optimal solution.
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] DittoTool commented on pull request #3926: Inefficient Usage of JCF
Posted by GitBox <gi...@apache.org>.
DittoTool commented on pull request #3926:
URL: https://github.com/apache/iotdb/pull/3926#issuecomment-914457304
I appended several other transformations just now, in which a list used for ownership checking is transformed to HashSet. I think these transformations are meaningful both in the theoretical and practical manner. Besides, here is another case as follows. https://github.com/apache/iotdb/blob/860c8de52ced88b4440f10bc0b07cd8a9c7441bd/jdbc/src/main/java/org/apache/iotdb/jdbc/IoTDBDatabaseMetadata.java#L1395-L1397
I did not include it and submit the transformation for it because the size of the list is only 4. However, I still think it would be better if a set is constructed to maintain the four elements rather than list during the initialization.
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] coveralls edited a comment on pull request #3926: Inefficient Usage of JCF
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3926:
URL: https://github.com/apache/iotdb/pull/3926#issuecomment-914365215
[](https://coveralls.io/builds/42704445)
Coverage decreased (-0.02%) to 67.402% when pulling **2d6fe1154114bed3fe0cc7be1d5eaef30be70088 on DittoTool:master** into **aaa96b974becf7cc57ef5ac36c3fa9e199423010 on apache:master**.
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] coveralls edited a comment on pull request #3926: [ISSUE-3927] Inefficient Usage of JCF
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3926:
URL: https://github.com/apache/iotdb/pull/3926#issuecomment-914365215
[](https://coveralls.io/builds/42762046)
Coverage increased (+0.09%) to 67.517% when pulling **3a1a84bba3ee4afa500bf776cf085c37df0cabac on DittoTool:master** into **aaa96b974becf7cc57ef5ac36c3fa9e199423010 on apache:master**.
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] Alima777 commented on pull request #3926: Inefficient Usage of JCF
Posted by GitBox <gi...@apache.org>.
Alima777 commented on pull request #3926:
URL: https://github.com/apache/iotdb/pull/3926#issuecomment-914867243
And you need to run `mvn:spotless apply` to modify the code style.
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] coveralls commented on pull request #3926: Inefficient Usage of JCF
Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #3926:
URL: https://github.com/apache/iotdb/pull/3926#issuecomment-914365215
[](https://coveralls.io/builds/42700755)
Coverage remained the same at 67.423% when pulling **ce266928e8519481ce60b0ad26e46be8ce4fe8f7 on DittoTool:master** into **aaa96b974becf7cc57ef5ac36c3fa9e199423010 on apache:master**.
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] Alima777 commented on pull request #3926: Inefficient Usage of JCF
Posted by GitBox <gi...@apache.org>.
Alima777 commented on pull request #3926:
URL: https://github.com/apache/iotdb/pull/3926#issuecomment-915022777
Hi, thank you for the discovery of the inefficient usage of jcf, almost all of them are correct. Since we often pay less attention to these details, if one analysis tool can be used to do this work, it will be great!
Thank you for the contribution! :D
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] SteveYurongSu merged pull request #3926: [ISSUE-3927] Inefficient Usage of JCF
Posted by GitBox <gi...@apache.org>.
SteveYurongSu merged pull request #3926:
URL: https://github.com/apache/iotdb/pull/3926
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] coveralls edited a comment on pull request #3926: [ISSUE-3927] Inefficient Usage of JCF
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3926:
URL: https://github.com/apache/iotdb/pull/3926#issuecomment-914365215
[](https://coveralls.io/builds/42722913)
Coverage increased (+0.09%) to 67.517% when pulling **a1f89ebe647c5c2c26c95185ea1504776feb39e1 on DittoTool:master** into **aaa96b974becf7cc57ef5ac36c3fa9e199423010 on apache:master**.
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] SteveYurongSu commented on a change in pull request #3926: Inefficient Usage of JCF
Posted by GitBox <gi...@apache.org>.
SteveYurongSu commented on a change in pull request #3926:
URL: https://github.com/apache/iotdb/pull/3926#discussion_r704147952
##########
File path: server/src/main/java/org/apache/iotdb/db/engine/storagegroup/TsFileProcessor.java
##########
@@ -746,7 +747,8 @@ public void syncFlush() throws IOException {
synchronized (tmpMemTable) {
try {
long startWait = System.currentTimeMillis();
- while (flushingMemTables.contains(tmpMemTable)) {
+ HashSet<IMemTable> flushingMemSet = new HashSet<>(flushingMemTables);
+ while (flushingMemSet.contains(tmpMemTable)) {
Review comment:
Theoretically speaking, the statement `flushingMemSet.contains(tmpMemTable)` is inefficient, so at least your finding is correct. 😊
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] coveralls edited a comment on pull request #3926: [ISSUE-3927] Inefficient Usage of JCF
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3926:
URL: https://github.com/apache/iotdb/pull/3926#issuecomment-914365215
[](https://coveralls.io/builds/42775347)
Coverage increased (+0.01%) to 67.432% when pulling **3a1a84bba3ee4afa500bf776cf085c37df0cabac on DittoTool:master** into **601df56b76df7dbda720361a536ab66e492f1b25 on apache:master**.
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] DittoTool edited a comment on pull request #3926: Inefficient Usage of JCF
Posted by GitBox <gi...@apache.org>.
DittoTool edited a comment on pull request #3926:
URL: https://github.com/apache/iotdb/pull/3926#issuecomment-914457304
I appended several other transformations just now, in which a list used for ownership checking is transformed to HashSet. I think these transformations are meaningful both in the theoretical and practical manner. Besides, here is another case as follows. https://github.com/apache/iotdb/blob/860c8de52ced88b4440f10bc0b07cd8a9c7441bd/jdbc/src/main/java/org/apache/iotdb/jdbc/IoTDBDatabaseMetadata.java#L1395-L1397
I did not include it and submit the transformation for it because the size of the list is only 4. However, I still think it would be better if a set was constructed to maintain the four elements rather than list during the initialization.
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] coveralls edited a comment on pull request #3926: [ISSUE-3927] Inefficient Usage of JCF
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3926:
URL: https://github.com/apache/iotdb/pull/3926#issuecomment-914365215
[](https://coveralls.io/builds/42893041)
Coverage decreased (-0.03%) to 67.39% when pulling **928ceb68a77040c3fbf59322fb8e9ea970cadb1b on DittoTool:master** into **601df56b76df7dbda720361a536ab66e492f1b25 on apache:master**.
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] coveralls edited a comment on pull request #3926: [ISSUE-3927] Inefficient Usage of JCF
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3926:
URL: https://github.com/apache/iotdb/pull/3926#issuecomment-914365215
[](https://coveralls.io/builds/42892911)
Coverage decreased (-0.04%) to 67.382% when pulling **928ceb68a77040c3fbf59322fb8e9ea970cadb1b on DittoTool:master** into **601df56b76df7dbda720361a536ab66e492f1b25 on apache:master**.
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] Alima777 commented on a change in pull request #3926: Inefficient Usage of JCF
Posted by GitBox <gi...@apache.org>.
Alima777 commented on a change in pull request #3926:
URL: https://github.com/apache/iotdb/pull/3926#discussion_r703988095
##########
File path: tsfile/src/main/java/org/apache/iotdb/tsfile/read/reader/chunk/VectorChunkReader.java
##########
@@ -59,7 +59,7 @@
protected Filter filter;
- private final List<IPageReader> pageReaderList = new LinkedList<>();
+ private final List<IPageReader> pageReaderList = new ArrayList<>();
Review comment:
The random access you mentioned maybe this...LinkedList is more efficient to remove the head element.
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] HTHou commented on a change in pull request #3926: [ISSUE-3927] Inefficient Usage of JCF
Posted by GitBox <gi...@apache.org>.
HTHou commented on a change in pull request #3926:
URL: https://github.com/apache/iotdb/pull/3926#discussion_r711522991
##########
File path: server/src/main/java/org/apache/iotdb/db/query/executor/fill/LastPointReader.java
##########
@@ -169,8 +171,14 @@ private TimeValuePair getChunkLastPoint(IChunkMetadata chunkMetaData) throws IOE
chunkStatistics.getEndTime(), chunkStatistics.getLastValue(), dataType);
}
List<IPageReader> pageReaders = FileLoaderUtils.loadPageReaderList(chunkMetaData, timeFilter);
- for (int i = pageReaders.size() - 1; i >= 0; i--) {
- IPageReader pageReader = pageReaders.get(i);
+ if (pageReaders instanceof LinkedList) {
+ Iterator it = ((LinkedList<IPageReader>) pageReaders).descendingIterator();
+ } else {
+ // for singleton list
+ Iterator it = pageReaders.iterator();
+ }
+ while (it.hasNext()) {
+ IPageReader pageReader = (IPageReader) it.next();
Review comment:
```suggestion
Iterator it;
if (pageReaders instanceof LinkedList) {
it = ((LinkedList<IPageReader>) pageReaders).descendingIterator();
} else {
// for singleton list
it = pageReaders.iterator();
}
while (it.hasNext()) {
IPageReader pageReader = (IPageReader) it.next();
```
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] coveralls edited a comment on pull request #3926: Inefficient Usage of JCF
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3926:
URL: https://github.com/apache/iotdb/pull/3926#issuecomment-914365215
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] DittoTool commented on a change in pull request #3926: Inefficient Usage of JCF
Posted by GitBox <gi...@apache.org>.
DittoTool commented on a change in pull request #3926:
URL: https://github.com/apache/iotdb/pull/3926#discussion_r704142755
##########
File path: server/src/main/java/org/apache/iotdb/db/engine/storagegroup/TsFileProcessor.java
##########
@@ -746,7 +747,8 @@ public void syncFlush() throws IOException {
synchronized (tmpMemTable) {
try {
long startWait = System.currentTimeMillis();
- while (flushingMemTables.contains(tmpMemTable)) {
+ HashSet<IMemTable> flushingMemSet = new HashSet<>(flushingMemTables);
+ while (flushingMemSet.contains(tmpMemTable)) {
Review comment:
Ditto has not modeled the semantics of multi-threading. It is true that the transformation here is not sound. Ditto just finds that there exists a performance issue here. The method contains of a list is quite inefficient. Moreover, it is invoked frequently in this loop.
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] Alima777 commented on a change in pull request #3926: Inefficient Usage of JCF
Posted by GitBox <gi...@apache.org>.
Alima777 commented on a change in pull request #3926:
URL: https://github.com/apache/iotdb/pull/3926#discussion_r703982984
##########
File path: tsfile/src/main/java/org/apache/iotdb/tsfile/read/reader/chunk/VectorChunkReader.java
##########
@@ -59,7 +59,7 @@
protected Filter filter;
- private final List<IPageReader> pageReaderList = new LinkedList<>();
+ private final List<IPageReader> pageReaderList = new ArrayList<>();
Review comment:
In vectorChunkReader Line 268, `return pageReaderList.remove(0).getAllSatisfiedPageData();`
##########
File path: tsfile/src/main/java/org/apache/iotdb/tsfile/read/reader/chunk/ChunkReader.java
##########
@@ -55,7 +55,7 @@
protected Filter filter;
- private List<IPageReader> pageReaderList = new LinkedList<>();
+ private List<IPageReader> pageReaderList = new ArrayList<>();
Review comment:
In chunkReader Line 118, `return pageReaderList.remove(0).getAllSatisfiedPageData();`
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/log/manage/FilePartitionedSnapshotLogManager.java
##########
@@ -130,11 +133,11 @@ private void collectTsFilesAndFillTimeseriesSchemas(List<Integer> requiredSlots)
// 2.register the measurement
boolean slotExistsInPartition;
- List<Integer> slots = null;
+ HashSet<Integer> slots = null;
Review comment:
It's ok
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] coveralls edited a comment on pull request #3926: Inefficient Usage of JCF
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3926:
URL: https://github.com/apache/iotdb/pull/3926#issuecomment-914365215
[](https://coveralls.io/builds/42720703)
Coverage decreased (-0.04%) to 67.386% when pulling **204c7ffc8ea39c17a041fcb2f893253e39a887c1 on DittoTool:master** into **aaa96b974becf7cc57ef5ac36c3fa9e199423010 on apache:master**.
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org