You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@carbondata.apache.org by GitBox <gi...@apache.org> on 2020/02/24 10:16:43 UTC
[GitHub] [carbondata] jackylk opened a new pull request #3638: [WIP] Avoid
data copy in Writer
jackylk opened a new pull request #3638: [WIP] Avoid data copy in Writer
URL: https://github.com/apache/carbondata/pull/3638
### Why is this PR needed?
Currently there are 6 data copies during data write process, in CarbonFactDataHandlerColumnar.processDataRows
### What changes were proposed in this PR?
This PR avoid all data copy by using DirectByteBuffer
### Does this PR introduce any user interface change?
- No
### Is any new testcase added?
- Yes
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] jackylk commented on issue #3638: [CARBONDATA-3731]
Avoid data copy in Writer
Posted by GitBox <gi...@apache.org>.
jackylk commented on issue #3638: [CARBONDATA-3731] Avoid data copy in Writer
URL: https://github.com/apache/carbondata/pull/3638#issuecomment-594351151
@ajantha-bhat
```
CarbonRowDataWriterProcessorStepImpl.processBatch:
CarbonRow converted = convertRowWithoutRearrange(row);
TablePage.convertToColumnarAndAddToPages:
byte[] valueWithLength = addShortLengthToByteArray((byte[]) noDictAndComplex[i]); // alloc and copy
noDictDimensionPages[i].putData(rowId, valueWithLength); // safe copy to unsafe
HighCardDictDimensionIndexCodec.encodeIndexStorage:
byte[][] data = input.getByteArrayPage(); // unsafe copy to safe
byte[] flattened = ByteUtil.flatten(indexStorage.getDataPage()); // alloc and copy
SnappyCompressor.compressByte:
Snappy.rawCompress(unCompInput, unCompInput.length); // copy inside, shoud use rawCompress(Object input, int inputOffset, int inputLength, byte[] output, int outputOffset)
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3638: [WIP] Avoid
data copy in Writer
Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on issue #3638: [WIP] Avoid data copy in Writer
URL: https://github.com/apache/carbondata/pull/3638#issuecomment-592918540
Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/2239/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3638: [WIP] Avoid
data copy in Writer
Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on issue #3638: [WIP] Avoid data copy in Writer
URL: https://github.com/apache/carbondata/pull/3638#issuecomment-591237646
Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/2176/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] ajantha-bhat commented on issue #3638:
[CARBONDATA-3731] Avoid data copy in Writer
Posted by GitBox <gi...@apache.org>.
ajantha-bhat commented on issue #3638: [CARBONDATA-3731] Avoid data copy in Writer
URL: https://github.com/apache/carbondata/pull/3638#issuecomment-595205806
LGTM
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3638: [WIP] Avoid
data copy in Writer
Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on issue #3638: [WIP] Avoid data copy in Writer
URL: https://github.com/apache/carbondata/pull/3638#issuecomment-590797805
Build Success with Spark 2.4.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.4/463/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] ajantha-bhat commented on a change in pull request
#3638: [CARBONDATA-3731] Avoid data copy in Writer
Posted by GitBox <gi...@apache.org>.
ajantha-bhat commented on a change in pull request #3638: [CARBONDATA-3731] Avoid data copy in Writer
URL: https://github.com/apache/carbondata/pull/3638#discussion_r387467864
##########
File path: core/src/main/java/org/apache/carbondata/core/datastore/page/ColumnPage.java
##########
@@ -182,20 +190,18 @@ public static ColumnPage newPage(ColumnPageEncoderMeta columnPageEncoderMeta, in
dataType == LONG ||
dataType == DataTypes.FLOAT ||
dataType == DataTypes.DOUBLE) {
- instance = new UnsafeFixLengthColumnPage(
- new ColumnPageEncoderMeta(columnSpec, dataType, compressorName), pageSize);
+ instance = new UnsafeFixLengthColumnPage(columnPageEncoderMeta, pageSize);
} else if (dataType == DataTypes.TIMESTAMP) {
instance = new UnsafeFixLengthColumnPage(
new ColumnPageEncoderMeta(columnSpec, LONG, compressorName), pageSize);
} else if (DataTypes.isDecimal(dataType)) {
- instance = new UnsafeDecimalColumnPage(
- new ColumnPageEncoderMeta(columnSpec, dataType, compressorName), pageSize);
- } else if (dataType == DataTypes.STRING
- || dataType == BYTE_ARRAY
- || dataType == DataTypes.VARCHAR
- || dataType == DataTypes.BINARY) {
- instance = new UnsafeVarLengthColumnPage(
- new ColumnPageEncoderMeta(columnSpec, dataType, compressorName), pageSize);
+ instance = new UnsafeDecimalColumnPage(columnPageEncoderMeta, pageSize);
+ } else if (dataType == DataTypes.STRING ||
+ dataType == DataTypes.VARCHAR ||
+ dataType == DataTypes.BINARY) {
+ instance = new ByteBufferColumnPage(columnPageEncoderMeta, pageSize);
+ } else if (dataType == BYTE_ARRAY) {
+ instance = new UnsafeVarLengthColumnPage(columnPageEncoderMeta, pageSize);
Review comment:
This is also LV flow, I think we can use `ByteBufferColumnPage` for BYTE_ARRAY also
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3638:
[CARBONDATA-3731] Avoid data copy in Writer
Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on issue #3638: [CARBONDATA-3731] Avoid data copy in Writer
URL: https://github.com/apache/carbondata/pull/3638#issuecomment-594363382
Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/2304/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3638: [WIP] Avoid
data copy in Writer
Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on issue #3638: [WIP] Avoid data copy in Writer
URL: https://github.com/apache/carbondata/pull/3638#issuecomment-590786470
Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/2162/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3638:
[CARBONDATA-3731] Avoid data copy in Writer
Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on issue #3638: [CARBONDATA-3731] Avoid data copy in Writer
URL: https://github.com/apache/carbondata/pull/3638#issuecomment-594419827
Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/2308/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] jackylk commented on a change in pull request #3638:
[CARBONDATA-3731] Avoid data copy in Writer
Posted by GitBox <gi...@apache.org>.
jackylk commented on a change in pull request #3638: [CARBONDATA-3731] Avoid data copy in Writer
URL: https://github.com/apache/carbondata/pull/3638#discussion_r387469426
##########
File path: core/src/main/java/org/apache/carbondata/core/datastore/compression/Compressor.java
##########
@@ -18,36 +18,39 @@
package org.apache.carbondata.core.datastore.compression;
import java.io.IOException;
+import java.nio.ByteBuffer;
public interface Compressor {
String getName();
- byte[] compressByte(byte[] unCompInput);
+ ByteBuffer compressByte(ByteBuffer compInput);
Review comment:
Many places need to change for query flow, can not do in this PR
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3638:
[CARBONDATA-3731] Avoid data copy in Writer
Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on issue #3638: [CARBONDATA-3731] Avoid data copy in Writer
URL: https://github.com/apache/carbondata/pull/3638#issuecomment-593814117
Build Success with Spark 2.4.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.4/572/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] jackylk edited a comment on issue #3638:
[CARBONDATA-3731] Avoid data copy in Writer
Posted by GitBox <gi...@apache.org>.
jackylk edited a comment on issue #3638: [CARBONDATA-3731] Avoid data copy in Writer
URL: https://github.com/apache/carbondata/pull/3638#issuecomment-594351151
@ajantha-bhat
In original code, 5 copies are:
```
TablePage.convertToColumnarAndAddToPages:
byte[] valueWithLength = addShortLengthToByteArray((byte[]) noDictAndComplex[i]); // alloc and copy
noDictDimensionPages[i].putData(rowId, valueWithLength); // safe copy to unsafe
HighCardDictDimensionIndexCodec.encodeIndexStorage:
byte[][] data = input.getByteArrayPage(); // copy unsafe to safe
byte[] flattened = ByteUtil.flatten(indexStorage.getDataPage()); // alloc and copy
SnappyCompressor.compressByte:
Snappy.rawCompress(unCompInput, unCompInput.length); // one copy and one compression write inside, should use direct buffer version of the API which has only one compression write
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3638: [WIP] Avoid
data copy in Writer
Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on issue #3638: [WIP] Avoid data copy in Writer
URL: https://github.com/apache/carbondata/pull/3638#issuecomment-592916688
Build Failed with Spark 2.4.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.4/538/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3638: [WIP] Avoid
data copy in Writer
Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on issue #3638: [WIP] Avoid data copy in Writer
URL: https://github.com/apache/carbondata/pull/3638#issuecomment-590338695
Build Failed with Spark 2.4.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.4/435/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3638: [WIP] Avoid
data copy in Writer
Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on issue #3638: [WIP] Avoid data copy in Writer
URL: https://github.com/apache/carbondata/pull/3638#issuecomment-590251925
Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/2128/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] jackylk commented on a change in pull request #3638:
[CARBONDATA-3731] Avoid data copy in Writer
Posted by GitBox <gi...@apache.org>.
jackylk commented on a change in pull request #3638: [CARBONDATA-3731] Avoid data copy in Writer
URL: https://github.com/apache/carbondata/pull/3638#discussion_r387470422
##########
File path: core/src/main/java/org/apache/carbondata/core/datastore/page/ColumnPage.java
##########
@@ -182,20 +190,18 @@ public static ColumnPage newPage(ColumnPageEncoderMeta columnPageEncoderMeta, in
dataType == LONG ||
dataType == DataTypes.FLOAT ||
dataType == DataTypes.DOUBLE) {
- instance = new UnsafeFixLengthColumnPage(
- new ColumnPageEncoderMeta(columnSpec, dataType, compressorName), pageSize);
+ instance = new UnsafeFixLengthColumnPage(columnPageEncoderMeta, pageSize);
} else if (dataType == DataTypes.TIMESTAMP) {
instance = new UnsafeFixLengthColumnPage(
new ColumnPageEncoderMeta(columnSpec, LONG, compressorName), pageSize);
} else if (DataTypes.isDecimal(dataType)) {
- instance = new UnsafeDecimalColumnPage(
- new ColumnPageEncoderMeta(columnSpec, dataType, compressorName), pageSize);
- } else if (dataType == DataTypes.STRING
- || dataType == BYTE_ARRAY
- || dataType == DataTypes.VARCHAR
- || dataType == DataTypes.BINARY) {
- instance = new UnsafeVarLengthColumnPage(
- new ColumnPageEncoderMeta(columnSpec, dataType, compressorName), pageSize);
+ instance = new UnsafeDecimalColumnPage(columnPageEncoderMeta, pageSize);
+ } else if (dataType == DataTypes.STRING ||
+ dataType == DataTypes.VARCHAR ||
+ dataType == DataTypes.BINARY) {
+ instance = new ByteBufferColumnPage(columnPageEncoderMeta, pageSize);
+ } else if (dataType == BYTE_ARRAY) {
+ instance = new UnsafeVarLengthColumnPage(columnPageEncoderMeta, pageSize);
Review comment:
It is not always for LV flow, for example, for local dictionary it uses UnsafeFixLengthColumnPage with store data type as BYTE_ARRAY
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3638: [WIP] Avoid
data copy in Writer
Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on issue #3638: [WIP] Avoid data copy in Writer
URL: https://github.com/apache/carbondata/pull/3638#issuecomment-590389915
Build Failed with Spark 2.4.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.4/437/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] jackylk commented on a change in pull request #3638:
[CARBONDATA-3731] Avoid data copy in Writer
Posted by GitBox <gi...@apache.org>.
jackylk commented on a change in pull request #3638: [CARBONDATA-3731] Avoid data copy in Writer
URL: https://github.com/apache/carbondata/pull/3638#discussion_r387469248
##########
File path: core/src/main/java/org/apache/carbondata/core/datastore/compression/AbstractCompressor.java
##########
@@ -48,10 +49,11 @@
}
@Override
- public byte[] compressInt(int[] unCompInput) {
- ByteBuffer unCompBuffer = ByteBuffer.allocate(unCompInput.length * ByteUtil.SIZEOF_INT);
+ public ByteBuffer compressInt(int[] unCompInput) {
+ ByteBuffer unCompBuffer = ByteBuffer.allocateDirect(unCompInput.length * ByteUtil.SIZEOF_INT);
Review comment:
DirectByteBuffer will be GC collected if there are no object reference to it. In our compression case, this object is temporary. So I think it is ok. @ajantha-bhat
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3638: [WIP] Avoid
data copy in Writer
Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on issue #3638: [WIP] Avoid data copy in Writer
URL: https://github.com/apache/carbondata/pull/3638#issuecomment-591226481
Build Failed with Spark 2.4.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.4/477/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3638:
[CARBONDATA-3731] Avoid data copy in Writer
Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on issue #3638: [CARBONDATA-3731] Avoid data copy in Writer
URL: https://github.com/apache/carbondata/pull/3638#issuecomment-594415233
Build Success with Spark 2.4.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.4/601/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3638: [WIP] Avoid
data copy in Writer
Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on issue #3638: [WIP] Avoid data copy in Writer
URL: https://github.com/apache/carbondata/pull/3638#issuecomment-590791016
Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/2164/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] asfgit closed pull request #3638: [CARBONDATA-3731]
Avoid data copy in Writer
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #3638: [CARBONDATA-3731] Avoid data copy in Writer
URL: https://github.com/apache/carbondata/pull/3638
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3638: [WIP] Avoid
data copy in Writer
Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on issue #3638: [WIP] Avoid data copy in Writer
URL: https://github.com/apache/carbondata/pull/3638#issuecomment-591347412
Build Success with Spark 2.4.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.4/497/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] jackylk commented on a change in pull request #3638:
[CARBONDATA-3731] Avoid data copy in Writer
Posted by GitBox <gi...@apache.org>.
jackylk commented on a change in pull request #3638: [CARBONDATA-3731] Avoid data copy in Writer
URL: https://github.com/apache/carbondata/pull/3638#discussion_r387470463
##########
File path: core/src/main/java/org/apache/carbondata/core/datastore/page/ColumnPage.java
##########
@@ -143,10 +144,17 @@ public static ColumnPage newLocalDictPage(ColumnPageEncoderMeta columnPageEncode
boolean isDecoderBasedFallBackEnabled = Boolean.parseBoolean(CarbonProperties.getInstance()
.getProperty(CarbonCommonConstants.LOCAL_DICTIONARY_DECODER_BASED_FALLBACK,
CarbonCommonConstants.LOCAL_DICTIONARY_DECODER_BASED_FALLBACK_DEFAULT));
+ DataType dataType = columnPageEncoderMeta.getStoreDataType();
ColumnPage actualPage;
ColumnPage encodedPage;
if (isUnsafeEnabled(columnPageEncoderMeta)) {
- actualPage = new UnsafeVarLengthColumnPage(columnPageEncoderMeta, pageSize);
+ if (dataType == DataTypes.STRING ||
+ dataType == DataTypes.VARCHAR ||
+ dataType == DataTypes.BINARY) {
+ actualPage = new ByteBufferColumnPage(columnPageEncoderMeta, pageSize);
+ } else {
+ actualPage = new UnsafeVarLengthColumnPage(columnPageEncoderMeta, pageSize);
Review comment:
Not really, complex column needs this
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3638: [WIP] Avoid
data copy in Writer
Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on issue #3638: [WIP] Avoid data copy in Writer
URL: https://github.com/apache/carbondata/pull/3638#issuecomment-590788006
Build Failed with Spark 2.4.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.4/461/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3638: [WIP] Avoid
data copy in Writer
Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on issue #3638: [WIP] Avoid data copy in Writer
URL: https://github.com/apache/carbondata/pull/3638#issuecomment-591370329
Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/2196/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] jackylk commented on a change in pull request #3638:
[CARBONDATA-3731] Avoid data copy in Writer
Posted by GitBox <gi...@apache.org>.
jackylk commented on a change in pull request #3638: [CARBONDATA-3731] Avoid data copy in Writer
URL: https://github.com/apache/carbondata/pull/3638#discussion_r387470537
##########
File path: core/src/main/java/org/apache/carbondata/core/datastore/page/ColumnPage.java
##########
@@ -218,10 +224,11 @@ public static ColumnPage newPage(ColumnPageEncoderMeta columnPageEncoderMeta, in
instance = newDoublePage(columnPageEncoderMeta, new double[pageSize]);
} else if (DataTypes.isDecimal(dataType)) {
instance = newDecimalPage(columnPageEncoderMeta, new byte[pageSize][]);
- } else if (dataType == DataTypes.STRING
- || dataType == BYTE_ARRAY
- || dataType == DataTypes.VARCHAR
- || dataType == DataTypes.BINARY) {
+ } else if (dataType == DataTypes.STRING ||
+ dataType == DataTypes.VARCHAR ||
+ dataType == DataTypes.BINARY) {
+ instance = new ByteBufferColumnPage(columnPageEncoderMeta, pageSize);
+ } else if (dataType == BYTE_ARRAY) {
Review comment:
It is not always for LV flow, for example, for local dictionary it uses UnsafeFixLengthColumnPage with store data type as BYTE_ARRAY
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] ajantha-bhat commented on a change in pull request
#3638: [CARBONDATA-3731] Avoid data copy in Writer
Posted by GitBox <gi...@apache.org>.
ajantha-bhat commented on a change in pull request #3638: [CARBONDATA-3731] Avoid data copy in Writer
URL: https://github.com/apache/carbondata/pull/3638#discussion_r387465206
##########
File path: core/src/main/java/org/apache/carbondata/core/datastore/compression/Compressor.java
##########
@@ -18,36 +18,39 @@
package org.apache.carbondata.core.datastore.compression;
import java.io.IOException;
+import java.nio.ByteBuffer;
public interface Compressor {
String getName();
- byte[] compressByte(byte[] unCompInput);
+ ByteBuffer compressByte(ByteBuffer compInput);
Review comment:
Similar problem exists in query flow also ? can we use `ByteBuffer` for `uncompress` return type also ?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3638:
[CARBONDATA-3731] Avoid data copy in Writer
Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on issue #3638: [CARBONDATA-3731] Avoid data copy in Writer
URL: https://github.com/apache/carbondata/pull/3638#issuecomment-594364130
Build Failed with Spark 2.4.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.4/597/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3638: [WIP] Avoid
data copy in Writer
Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on issue #3638: [WIP] Avoid data copy in Writer
URL: https://github.com/apache/carbondata/pull/3638#issuecomment-590406286
Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/2138/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] jackylk edited a comment on issue #3638:
[CARBONDATA-3731] Avoid data copy in Writer
Posted by GitBox <gi...@apache.org>.
jackylk edited a comment on issue #3638: [CARBONDATA-3731] Avoid data copy in Writer
URL: https://github.com/apache/carbondata/pull/3638#issuecomment-594351151
@ajantha-bhat
In original code, 5 copies are:
```
TablePage.convertToColumnarAndAddToPages:
byte[] valueWithLength = addShortLengthToByteArray((byte[]) noDictAndComplex[i]); // alloc and copy
noDictDimensionPages[i].putData(rowId, valueWithLength); // safe copy to unsafe
HighCardDictDimensionIndexCodec.encodeIndexStorage:
byte[][] data = input.getByteArrayPage(); // copy unsafe to safe
byte[] flattened = ByteUtil.flatten(indexStorage.getDataPage()); // alloc and copy
SnappyCompressor.compressByte:
Snappy.rawCompress(unCompInput, unCompInput.length); // one copy and one compression write inside, shoud use direct buffer which has only one compression write
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] ajantha-bhat commented on issue #3638:
[CARBONDATA-3731] Avoid data copy in Writer
Posted by GitBox <gi...@apache.org>.
ajantha-bhat commented on issue #3638: [CARBONDATA-3731] Avoid data copy in Writer
URL: https://github.com/apache/carbondata/pull/3638#issuecomment-594350791
I have gone through on a very high level. I need to check out code and review agian.
@jackylk : It will be useful if you can give one flow of all the 5 places where new data was created. I couldn't find 5 places.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] jackylk commented on a change in pull request #3638:
[CARBONDATA-3731] Avoid data copy in Writer
Posted by GitBox <gi...@apache.org>.
jackylk commented on a change in pull request #3638: [CARBONDATA-3731] Avoid data copy in Writer
URL: https://github.com/apache/carbondata/pull/3638#discussion_r387470463
##########
File path: core/src/main/java/org/apache/carbondata/core/datastore/page/ColumnPage.java
##########
@@ -143,10 +144,17 @@ public static ColumnPage newLocalDictPage(ColumnPageEncoderMeta columnPageEncode
boolean isDecoderBasedFallBackEnabled = Boolean.parseBoolean(CarbonProperties.getInstance()
.getProperty(CarbonCommonConstants.LOCAL_DICTIONARY_DECODER_BASED_FALLBACK,
CarbonCommonConstants.LOCAL_DICTIONARY_DECODER_BASED_FALLBACK_DEFAULT));
+ DataType dataType = columnPageEncoderMeta.getStoreDataType();
ColumnPage actualPage;
ColumnPage encodedPage;
if (isUnsafeEnabled(columnPageEncoderMeta)) {
- actualPage = new UnsafeVarLengthColumnPage(columnPageEncoderMeta, pageSize);
+ if (dataType == DataTypes.STRING ||
+ dataType == DataTypes.VARCHAR ||
+ dataType == DataTypes.BINARY) {
+ actualPage = new ByteBufferColumnPage(columnPageEncoderMeta, pageSize);
+ } else {
+ actualPage = new UnsafeVarLengthColumnPage(columnPageEncoderMeta, pageSize);
Review comment:
removed
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3638: [WIP] Avoid
data copy in Writer
Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on issue #3638: [WIP] Avoid data copy in Writer
URL: https://github.com/apache/carbondata/pull/3638#issuecomment-590412292
Build Success with Spark 2.4.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.4/438/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] ajantha-bhat commented on a change in pull request
#3638: [CARBONDATA-3731] Avoid data copy in Writer
Posted by GitBox <gi...@apache.org>.
ajantha-bhat commented on a change in pull request #3638: [CARBONDATA-3731] Avoid data copy in Writer
URL: https://github.com/apache/carbondata/pull/3638#discussion_r387467961
##########
File path: core/src/main/java/org/apache/carbondata/core/datastore/page/ColumnPage.java
##########
@@ -218,10 +224,11 @@ public static ColumnPage newPage(ColumnPageEncoderMeta columnPageEncoderMeta, in
instance = newDoublePage(columnPageEncoderMeta, new double[pageSize]);
} else if (DataTypes.isDecimal(dataType)) {
instance = newDecimalPage(columnPageEncoderMeta, new byte[pageSize][]);
- } else if (dataType == DataTypes.STRING
- || dataType == BYTE_ARRAY
- || dataType == DataTypes.VARCHAR
- || dataType == DataTypes.BINARY) {
+ } else if (dataType == DataTypes.STRING ||
+ dataType == DataTypes.VARCHAR ||
+ dataType == DataTypes.BINARY) {
+ instance = new ByteBufferColumnPage(columnPageEncoderMeta, pageSize);
+ } else if (dataType == BYTE_ARRAY) {
Review comment:
same as above
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] jackylk edited a comment on issue #3638:
[CARBONDATA-3731] Avoid data copy in Writer
Posted by GitBox <gi...@apache.org>.
jackylk edited a comment on issue #3638: [CARBONDATA-3731] Avoid data copy in Writer
URL: https://github.com/apache/carbondata/pull/3638#issuecomment-594351151
@ajantha-bhat
In original code, 5 copies are:
```
TablePage.convertToColumnarAndAddToPages:
byte[] valueWithLength = addShortLengthToByteArray((byte[]) noDictAndComplex[i]); // alloc and copy
noDictDimensionPages[i].putData(rowId, valueWithLength); // copy safe to unsafe
HighCardDictDimensionIndexCodec.encodeIndexStorage:
byte[][] data = input.getByteArrayPage(); // copy unsafe to safe
byte[] flattened = ByteUtil.flatten(indexStorage.getDataPage()); // alloc and copy
SnappyCompressor.compressByte:
Snappy.rawCompress(unCompInput, unCompInput.length); // one copy and one compression write inside, should use direct buffer version of the API which has only one compression write
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3638:
[CARBONDATA-3731] Avoid data copy in Writer
Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on issue #3638: [CARBONDATA-3731] Avoid data copy in Writer
URL: https://github.com/apache/carbondata/pull/3638#issuecomment-593843924
Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/2277/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] ajantha-bhat commented on a change in pull request
#3638: [CARBONDATA-3731] Avoid data copy in Writer
Posted by GitBox <gi...@apache.org>.
ajantha-bhat commented on a change in pull request #3638: [CARBONDATA-3731] Avoid data copy in Writer
URL: https://github.com/apache/carbondata/pull/3638#discussion_r387467031
##########
File path: core/src/main/java/org/apache/carbondata/core/datastore/page/ColumnPage.java
##########
@@ -143,10 +144,17 @@ public static ColumnPage newLocalDictPage(ColumnPageEncoderMeta columnPageEncode
boolean isDecoderBasedFallBackEnabled = Boolean.parseBoolean(CarbonProperties.getInstance()
.getProperty(CarbonCommonConstants.LOCAL_DICTIONARY_DECODER_BASED_FALLBACK,
CarbonCommonConstants.LOCAL_DICTIONARY_DECODER_BASED_FALLBACK_DEFAULT));
+ DataType dataType = columnPageEncoderMeta.getStoreDataType();
ColumnPage actualPage;
ColumnPage encodedPage;
if (isUnsafeEnabled(columnPageEncoderMeta)) {
- actualPage = new UnsafeVarLengthColumnPage(columnPageEncoderMeta, pageSize);
+ if (dataType == DataTypes.STRING ||
+ dataType == DataTypes.VARCHAR ||
+ dataType == DataTypes.BINARY) {
+ actualPage = new ByteBufferColumnPage(columnPageEncoderMeta, pageSize);
+ } else {
+ actualPage = new UnsafeVarLengthColumnPage(columnPageEncoderMeta, pageSize);
Review comment:
local dictionary supported for string and varchar. so this may be dead code now. remove it
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3638: [WIP] Avoid
data copy in Writer
Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on issue #3638: [WIP] Avoid data copy in Writer
URL: https://github.com/apache/carbondata/pull/3638#issuecomment-590251490
Build Failed with Spark 2.4.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.4/428/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] ajantha-bhat commented on a change in pull request
#3638: [CARBONDATA-3731] Avoid data copy in Writer
Posted by GitBox <gi...@apache.org>.
ajantha-bhat commented on a change in pull request #3638: [CARBONDATA-3731] Avoid data copy in Writer
URL: https://github.com/apache/carbondata/pull/3638#discussion_r387449577
##########
File path: core/src/main/java/org/apache/carbondata/core/datastore/compression/AbstractCompressor.java
##########
@@ -48,10 +49,11 @@
}
@Override
- public byte[] compressInt(int[] unCompInput) {
- ByteBuffer unCompBuffer = ByteBuffer.allocate(unCompInput.length * ByteUtil.SIZEOF_INT);
+ public ByteBuffer compressInt(int[] unCompInput) {
+ ByteBuffer unCompBuffer = ByteBuffer.allocateDirect(unCompInput.length * ByteUtil.SIZEOF_INT);
Review comment:
`DirecByteBuffer` are tricky, they reside outside JVM. we need to clean by reflection. Hence it is not used in our code. (can use unsafe if we still need to offheap memory)
DirecByteBuffers are useful only while interacting with native libraries.
So, I suggest we can use `ByteBuffer` instead of `DirectByteBuffer`
https://stackoverflow.com/questions/5670862/bytebuffer-allocate-vs-bytebuffer-allocatedirect
@ravipesala , @kumarvishal09 what's your opinion on this ?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3638: [WIP] Avoid
data copy in Writer
Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on issue #3638: [WIP] Avoid data copy in Writer
URL: https://github.com/apache/carbondata/pull/3638#issuecomment-590339219
Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/2135/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3638: [WIP] Avoid
data copy in Writer
Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on issue #3638: [WIP] Avoid data copy in Writer
URL: https://github.com/apache/carbondata/pull/3638#issuecomment-590390665
Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/2137/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services