You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@carbondata.apache.org by kumarvishal09 <gi...@git.apache.org> on 2018/10/29 07:41:20 UTC
[GitHub] carbondata pull request #2872: [WIP] Added reusable buffer code
GitHub user kumarvishal09 opened a pull request:
https://github.com/apache/carbondata/pull/2872
[WIP] Added reusable buffer code
Be sure to do all of the following checklist to help us incorporate
your contribution quickly and easily:
- [ ] Any interfaces changed?
- [ ] Any backward compatibility impacted?
- [ ] Document update required?
- [ ] Testing done
Please provide details on
- Whether new unit test cases have been added or why no new tests are required?
- How it is tested? Please attach test report.
- Is it a performance related change? Please attach the performance test report.
- Any additional information to help reviewers in testing this change.
- [ ] For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/kumarvishal09/incubator-carbondata 2863
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/carbondata/pull/2872.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #2872
----
commit d382d8d92e91a9b20a1b95dcdf4724ee21969b37
Author: ravipesala <ra...@...>
Date: 2018-10-26T15:20:53Z
Optimize decompressing while filling the vector during conversion of primitive types
commit bd00dfbab9208f3f82cdf5a357b57b0933285152
Author: kumarvishal09 <ku...@...>
Date: 2018-10-28T15:09:05Z
reusable byteArray
----
---
[GitHub] carbondata pull request #2872: [WIP] Added reusable buffer code
Posted by xuchuanyin <gi...@git.apache.org>.
Github user xuchuanyin commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2872#discussion_r228902627
--- Diff: core/src/main/java/org/apache/carbondata/core/datastore/page/ColumnPageByteUtil.java ---
@@ -0,0 +1,49 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.carbondata.core.datastore.page;
+
+/**
+ * Utility methods to converts to primitive types only column page data decode.
+ */
+public class ColumnPageByteUtil {
--- End diff --
What's the difference between this and `ByteUtil.java`?
I see the same name of function with different implementations, please consider to optimize and add some notes.
---
[GitHub] carbondata issue #2872: [WIP] Added reusable buffer code
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2872
Build Failed with Spark 2.3.1, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/9375/
---
[GitHub] carbondata issue #2872: [WIP] Added reusable buffer code
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2872
Build Failed with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/1423/
---
[GitHub] carbondata issue #2872: [WIP] Added reusable buffer code
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2872
Build Failed with Spark 2.3.1, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/9528/
---
[GitHub] carbondata issue #2872: [WIP] Added reusable buffer code
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2872
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1388/
---
[GitHub] carbondata issue #2872: [WIP] Added reusable buffer code
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2872
Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1207/
---
[GitHub] carbondata issue #2872: [WIP] Added reusable buffer code
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2872
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1389/
---
[GitHub] carbondata issue #2872: [CARBONDATA-3113] Fixed Local Dictionary Query Perfo...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2872
Build Success with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/1688/
---
[GitHub] carbondata issue #2872: [WIP] Added reusable buffer code
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2872
Build Failed with Spark 2.3.1, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/9515/
---
[GitHub] carbondata issue #2872: [CARBONDATA-3113] Fixed Local Dictionary Query Perfo...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2872
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1478/
---
[GitHub] carbondata issue #2872: [WIP] Added reusable buffer code
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2872
Build Failed with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/1327/
---
[GitHub] carbondata pull request #2872: [WIP] Added reusable buffer code
Posted by kumarvishal09 <gi...@git.apache.org>.
Github user kumarvishal09 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2872#discussion_r235060209
--- Diff: core/src/main/java/org/apache/carbondata/core/datastore/chunk/reader/MeasureColumnChunkReader.java ---
@@ -56,13 +57,14 @@ MeasureRawColumnChunk readRawMeasureChunk(FileReader fileReader, int columnIndex
* @return
* @throws IOException
*/
- ColumnPage decodeColumnPage(MeasureRawColumnChunk measureRawColumnChunk,
- int pageNumber) throws IOException, MemoryException;
+ ColumnPage decodeColumnPage(MeasureRawColumnChunk measureRawColumnChunk, int pageNumber,
+ ReusableDataBuffer reusableDataBuffer) throws IOException, MemoryException;
/**
* Decode raw data and fill the vector
*/
- void decodeColumnPageAndFillVector(MeasureRawColumnChunk measureRawColumnChunk,
- int pageNumber, ColumnVectorInfo vectorInfo) throws IOException, MemoryException;
+ void decodeColumnPageAndFillVector(MeasureRawColumnChunk measureRawColumnChunk, int pageNumber,
+ ColumnVectorInfo vectorInfo, ReusableDataBuffer reusableDataBuffer)
+ throws IOException, MemoryException;
--- End diff --
I have remove resuableBuffer from other two method for now as Reusable buffer code was only added for direct fill (new flow)
---
[GitHub] carbondata issue #2872: [WIP] Added reusable buffer code
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2872
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1116/
---
[GitHub] carbondata issue #2872: [WIP] Added reusable buffer code
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2872
Build Failed with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/1670/
---
[GitHub] carbondata issue #2872: [WIP] Added reusable buffer code
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2872
Build Success with Spark 2.3.1, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/9674/
---
[GitHub] carbondata issue #2872: [WIP] Added reusable buffer code
Posted by kumarvishal09 <gi...@git.apache.org>.
Github user kumarvishal09 commented on the issue:
https://github.com/apache/carbondata/pull/2872
retest this please
---
[GitHub] carbondata issue #2872: [CARBONDATA-3113] Fixed Local Dictionary Query Perfo...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2872
Build Failed with Spark 2.3.1, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/9734/
---
[GitHub] carbondata issue #2872: [WIP] Added reusable buffer code
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2872
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1392/
---
[GitHub] carbondata issue #2872: [WIP] Added reusable buffer code
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2872
Build Success with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/1678/
---
[GitHub] carbondata pull request #2872: [WIP] Added reusable buffer code
Posted by kumarvishal09 <gi...@git.apache.org>.
Github user kumarvishal09 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2872#discussion_r228915419
--- Diff: core/src/main/java/org/apache/carbondata/core/datastore/page/ColumnPageByteUtil.java ---
@@ -0,0 +1,49 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.carbondata.core.datastore.page;
+
+/**
+ * Utility methods to converts to primitive types only column page data decode.
+ */
+public class ColumnPageByteUtil {
--- End diff --
@xuchuanyin This PR depends on 2863
---
[GitHub] carbondata pull request #2872: [WIP] Added reusable buffer code
Posted by xuchuanyin <gi...@git.apache.org>.
Github user xuchuanyin commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2872#discussion_r228901408
--- Diff: core/src/main/java/org/apache/carbondata/core/datastore/compression/ZstdCompressor.java ---
@@ -167,4 +167,12 @@ public long maxCompressedLength(long inputSize) {
public boolean supportUnsafe() {
return false;
}
+
+ @Override public int uncompressedLength(byte[] data, int offset, int length) {
--- End diff --
Why this is unsupported since this is supported in Zstd
```
/**
* Return the original size of a compressed buffer (if known)
*
* @param src the compressed buffer
* @return the number of bytes of the original buffer
* 0 if the original size is now known
*/
public static native long decompressedSize(byte[] src);
```
---
[GitHub] carbondata pull request #2872: [WIP] Added reusable buffer code
Posted by jackylk <gi...@git.apache.org>.
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2872#discussion_r234209700
--- Diff: core/src/main/java/org/apache/carbondata/core/datastore/ReusableDataBuffer.java ---
@@ -0,0 +1,33 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.carbondata.core.datastore;
+
+public class ReusableDataBuffer {
--- End diff --
add interface annotation also
---
[GitHub] carbondata issue #2872: [WIP] Added reusable buffer code
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2872
Build Failed with Spark 2.3.1, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/9645/
---
[GitHub] carbondata issue #2872: [CARBONDATA-3113] Fixed Local Dictionary Query Perfo...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2872
Build Failed with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/1686/
---
[GitHub] carbondata issue #2872: [CARBONDATA-3113] Fixed Local Dictionary Query Perfo...
Posted by manishgupta88 <gi...@git.apache.org>.
Github user manishgupta88 commented on the issue:
https://github.com/apache/carbondata/pull/2872
LGTM
---
[GitHub] carbondata issue #2872: [WIP] Added reusable buffer code
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2872
Build Failed with Spark 2.3.1, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/9718/
---
[GitHub] carbondata issue #2872: [WIP] Added reusable buffer code
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2872
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1468/
---
[GitHub] carbondata issue #2872: [WIP] Added reusable buffer code
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2872
Build Failed with Spark 2.3.1, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/9379/
---
[GitHub] carbondata issue #2872: [WIP] Added reusable buffer code
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2872
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1417/
---
[GitHub] carbondata pull request #2872: [WIP] Added reusable buffer code
Posted by kumarvishal09 <gi...@git.apache.org>.
Github user kumarvishal09 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2872#discussion_r235059863
--- Diff: core/src/main/java/org/apache/carbondata/core/datastore/compression/ZstdCompressor.java ---
@@ -74,4 +74,16 @@ public long maxCompressedLength(long inputSize) {
public boolean supportUnsafe() {
return false;
}
+
+ @Override public int unCompressedLength(byte[] data, int offset, int length) {
+ throw new RuntimeException("Unsupported operation Exception");
+ }
+
+ @Override public int rawUncompress(byte[] data, int offset, int length, byte[] output) {
+ throw new RuntimeException("Unsupported operation Exception");
+ }
+
+ @Override public boolean supportReusableBuffer() {
+ return false;
--- End diff --
ok
---
[GitHub] carbondata issue #2872: [WIP] Added reusable buffer code
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2872
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1113/
---
[GitHub] carbondata pull request #2872: [WIP] Added reusable buffer code
Posted by manishgupta88 <gi...@git.apache.org>.
Github user manishgupta88 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2872#discussion_r234201988
--- Diff: core/src/main/java/org/apache/carbondata/core/datastore/compression/ZstdCompressor.java ---
@@ -74,4 +74,16 @@ public long maxCompressedLength(long inputSize) {
public boolean supportUnsafe() {
return false;
}
+
+ @Override public int unCompressedLength(byte[] data, int offset, int length) {
+ throw new RuntimeException("Unsupported operation Exception");
+ }
+
+ @Override public int rawUncompress(byte[] data, int offset, int length, byte[] output) {
+ throw new RuntimeException("Unsupported operation Exception");
+ }
+
+ @Override public boolean supportReusableBuffer() {
+ return false;
--- End diff --
Move the default implementation to Abstract class `AbstractCompressor`
---
[GitHub] carbondata issue #2872: [WIP] Added reusable buffer code
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2872
Build Failed with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/1321/
---
[GitHub] carbondata issue #2872: [WIP] Added reusable buffer code
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2872
Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1110/
---
[GitHub] carbondata issue #2872: [WIP] Added reusable buffer code
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2872
Build Failed with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/1466/
---
[GitHub] carbondata issue #2872: [WIP] Added reusable buffer code
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2872
Build Failed with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/1608/
---
[GitHub] carbondata pull request #2872: [WIP] Added reusable buffer code
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2872#discussion_r234125610
--- Diff: core/src/main/java/org/apache/carbondata/core/datastore/ReusableDataBuffer.java ---
@@ -0,0 +1,33 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.carbondata.core.datastore;
+
+public class ReusableDataBuffer {
--- End diff --
Please add comments to this class
---
[GitHub] carbondata issue #2872: [WIP] Added reusable buffer code
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2872
Build Success with Spark 2.3.1, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/9721/
---
[GitHub] carbondata pull request #2872: [WIP] Added reusable buffer code
Posted by jackylk <gi...@git.apache.org>.
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2872#discussion_r234209828
--- Diff: core/src/main/java/org/apache/carbondata/core/datastore/ReusableDataBuffer.java ---
@@ -0,0 +1,33 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.carbondata.core.datastore;
+
+public class ReusableDataBuffer {
+
+ private byte[] dataBuffer;
+
+ private int size;
+
+ public byte[] getDataBuffer(int requestedSize) {
--- End diff --
add comment
---
[GitHub] carbondata issue #2872: [WIP] Added reusable buffer code
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2872
Build Failed with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/1422/
---
[GitHub] carbondata issue #2872: [WIP] Added reusable buffer code
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2872
Build Failed with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/1597/
---
[GitHub] carbondata issue #2872: [WIP] Added reusable buffer code
Posted by jackylk <gi...@git.apache.org>.
Github user jackylk commented on the issue:
https://github.com/apache/carbondata/pull/2872
LGTM
please change the title to incluce jira ticket number
---
[GitHub] carbondata pull request #2872: [WIP] Added reusable buffer code
Posted by kumarvishal09 <gi...@git.apache.org>.
Github user kumarvishal09 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2872#discussion_r235059822
--- Diff: core/src/main/java/org/apache/carbondata/core/datastore/ReusableDataBuffer.java ---
@@ -0,0 +1,33 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.carbondata.core.datastore;
+
+public class ReusableDataBuffer {
+
+ private byte[] dataBuffer;
+
+ private int size;
+
+ public byte[] getDataBuffer(int requestedSize) {
--- End diff --
ok
---
[GitHub] carbondata issue #2872: [WIP] Added reusable buffer code
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2872
Build Success with Spark 2.3.1, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/9647/
---
[GitHub] carbondata issue #2872: [WIP] Added reusable buffer code
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2872
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1398/
---
[GitHub] carbondata issue #2872: [WIP] Added reusable buffer code
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2872
Build Failed with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/1338/
---
[GitHub] carbondata issue #2872: [WIP] Added reusable buffer code
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2872
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1128/
---
[GitHub] carbondata pull request #2872: [CARBONDATA-3113] Fixed Local Dictionary Quer...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/carbondata/pull/2872
---
[GitHub] carbondata pull request #2872: [WIP] Added reusable buffer code
Posted by kumarvishal09 <gi...@git.apache.org>.
Github user kumarvishal09 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2872#discussion_r235059766
--- Diff: core/src/main/java/org/apache/carbondata/core/datastore/page/encoding/adaptive/AdaptiveIntegralCodec.java ---
@@ -119,24 +120,34 @@ public ColumnPage decode(byte[] input, int offset, int length)
return LazyColumnPage.newPage(page, converter);
}
- @Override
- public void decodeAndFillVector(byte[] input, int offset, int length,
- ColumnVectorInfo vectorInfo, BitSet nullBits, boolean isLVEncoded)
- throws MemoryException, IOException {
- ColumnPage page = null;
- if (DataTypes.isDecimal(meta.getSchemaDataType())) {
- page = ColumnPage.decompressDecimalPage(meta, input, offset, length);
- vectorInfo.decimalConverter = ((DecimalColumnPage) page).getDecimalConverter();
+ @Override public void decodeAndFillVector(byte[] input, int offset, int length,
+ ColumnVectorInfo vectorInfo, BitSet nullBits, boolean isLVEncoded, int pageSize,
+ ReusableDataBuffer reusableDataBuffer) throws MemoryException, IOException {
+ Compressor compressor =
+ CompressorFactory.getInstance().getCompressor(meta.getCompressorName());
+ byte[] unCompressData;
+ if (null != reusableDataBuffer && compressor.supportReusableBuffer()) {
+ int uncompressedLength = compressor.unCompressedLength(input, offset, length);
+ unCompressData = reusableDataBuffer.getDataBuffer(uncompressedLength);
} else {
- page = ColumnPage.decompress(meta, input, offset, length, isLVEncoded);
+ unCompressData = compressor.unCompressByte(input, offset, length);
}
- page.setNullBits(nullBits);
- converter.decodeAndFillVector(page, vectorInfo);
+ compressor.rawUncompress(input, offset, length, unCompressData);
--- End diff --
Fixed
---
[GitHub] carbondata issue #2872: [WIP] Added reusable buffer code
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2872
Build Failed with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/1599/
---
[GitHub] carbondata issue #2872: [WIP] Added reusable buffer code
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2872
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1264/
---
[GitHub] carbondata pull request #2872: [WIP] Added reusable buffer code
Posted by akashrn5 <gi...@git.apache.org>.
Github user akashrn5 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2872#discussion_r234233366
--- Diff: core/src/main/java/org/apache/carbondata/core/datastore/page/encoding/adaptive/AdaptiveIntegralCodec.java ---
@@ -119,24 +120,34 @@ public ColumnPage decode(byte[] input, int offset, int length)
return LazyColumnPage.newPage(page, converter);
}
- @Override
- public void decodeAndFillVector(byte[] input, int offset, int length,
- ColumnVectorInfo vectorInfo, BitSet nullBits, boolean isLVEncoded)
- throws MemoryException, IOException {
- ColumnPage page = null;
- if (DataTypes.isDecimal(meta.getSchemaDataType())) {
- page = ColumnPage.decompressDecimalPage(meta, input, offset, length);
- vectorInfo.decimalConverter = ((DecimalColumnPage) page).getDecimalConverter();
+ @Override public void decodeAndFillVector(byte[] input, int offset, int length,
+ ColumnVectorInfo vectorInfo, BitSet nullBits, boolean isLVEncoded, int pageSize,
+ ReusableDataBuffer reusableDataBuffer) throws MemoryException, IOException {
+ Compressor compressor =
+ CompressorFactory.getInstance().getCompressor(meta.getCompressorName());
+ byte[] unCompressData;
+ if (null != reusableDataBuffer && compressor.supportReusableBuffer()) {
+ int uncompressedLength = compressor.unCompressedLength(input, offset, length);
+ unCompressData = reusableDataBuffer.getDataBuffer(uncompressedLength);
} else {
- page = ColumnPage.decompress(meta, input, offset, length, isLVEncoded);
+ unCompressData = compressor.unCompressByte(input, offset, length);
}
- page.setNullBits(nullBits);
- converter.decodeAndFillVector(page, vectorInfo);
+ compressor.rawUncompress(input, offset, length, unCompressData);
--- End diff --
i think, when ZSTD compressor is configured, it will throw unsupported exception, plese check and handle for `AdaptiveIntegralCodec` and `AdaptiveDeltaIntegralCodec`
---
[GitHub] carbondata issue #2872: [CARBONDATA-3113] Fixed Local Dictionary Query Perfo...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2872
Build Success with Spark 2.3.1, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/9736/
---
[GitHub] carbondata issue #2872: [WIP] Added reusable buffer code
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2872
Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1251/
---
[GitHub] carbondata issue #2872: [WIP] Added reusable buffer code
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2872
Build Success with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/1673/
---
[GitHub] carbondata issue #2872: [WIP] Added reusable buffer code
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2872
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1463/
---
[GitHub] carbondata issue #2872: [WIP] Added reusable buffer code
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2872
Build Failed with Spark 2.3.1, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/9392/
---
[GitHub] carbondata issue #2872: [WIP] Added reusable buffer code
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2872
Build Success with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/1626/
---
[GitHub] carbondata issue #2872: [WIP] Added reusable buffer code
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2872
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1387/
---
[GitHub] carbondata pull request #2872: [WIP] Added reusable buffer code
Posted by kumarvishal09 <gi...@git.apache.org>.
Github user kumarvishal09 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2872#discussion_r235060261
--- Diff: core/src/main/java/org/apache/carbondata/core/datastore/ReusableDataBuffer.java ---
@@ -0,0 +1,33 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.carbondata.core.datastore;
+
+public class ReusableDataBuffer {
--- End diff --
ok
---
[GitHub] carbondata issue #2872: [CARBONDATA-3113] Fixed Local Dictionary Query Perfo...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2872
Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1477/
---
[GitHub] carbondata issue #2872: [WIP] Added reusable buffer code
Posted by manishgupta88 <gi...@git.apache.org>.
Github user manishgupta88 commented on the issue:
https://github.com/apache/carbondata/pull/2872
@kumarvishal09 ...In general, instead of passing the `ReusableDataBuffer` instance everwhere can we make this class singleton and create a `byte[]` pool and use the concept of object pool. This will help in following ways:
1. It will avoid modification of too many method/code modifications as wherever required we can take the object from pool and make the code simpler.
2. Currentlt it is being used for filling hthe decompressed data to avoid array copy. In future it will be easier to make use of the reusable buffer object pool wherever required without making too many changes in the code.
We can discuss more on this and decide.
---
[GitHub] carbondata pull request #2872: [WIP] Added reusable buffer code
Posted by kumarvishal09 <gi...@git.apache.org>.
Github user kumarvishal09 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2872#discussion_r228915613
--- Diff: core/src/main/java/org/apache/carbondata/core/datastore/compression/ZstdCompressor.java ---
@@ -167,4 +167,12 @@ public long maxCompressedLength(long inputSize) {
public boolean supportUnsafe() {
return false;
}
+
+ @Override public int uncompressedLength(byte[] data, int offset, int length) {
--- End diff --
@xuchuanyin Still we are working on it. This is not final code
---
[GitHub] carbondata issue #2872: [WIP] Added reusable buffer code
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2872
Build Failed with Spark 2.3.1, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/9371/
---
[GitHub] carbondata pull request #2872: [WIP] Added reusable buffer code
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2872#discussion_r233321330
--- Diff: integration/spark-datasource/src/main/spark2.1andspark2.2/org/apache/spark/sql/CarbonVectorProxy.java ---
@@ -454,7 +458,11 @@ public ColumnVector reserveDictionaryIds(int capacity) {
* and offset.
*/
public void putAllByteArray(byte[] data, int offset, int length) {
- vector.arrayData().appendBytes(length, data, offset);
+ try {
+ byteArray.set(vector.arrayData(), data);
--- End diff --
please remove this
---
[GitHub] carbondata issue #2872: [WIP] Added reusable buffer code
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2872
Build Failed with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/1479/
---
[GitHub] carbondata issue #2872: [WIP] Added reusable buffer code
Posted by kumarvishal09 <gi...@git.apache.org>.
Github user kumarvishal09 commented on the issue:
https://github.com/apache/carbondata/pull/2872
@manishgupta88 this is basic implementation of pool(kind of pool :)) and currently it only for direct flow(new flow), In future I am planing to replace reusable buffer with proper byte array pooling which can be used for both old and new flow. I will raise a JIRA for the same
---
[GitHub] carbondata issue #2872: [WIP] Added reusable buffer code
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2872
Build Failed with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/1602/
---
[GitHub] carbondata pull request #2872: [WIP] Added reusable buffer code
Posted by manishgupta88 <gi...@git.apache.org>.
Github user manishgupta88 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2872#discussion_r234176956
--- Diff: core/src/main/java/org/apache/carbondata/core/datastore/chunk/reader/MeasureColumnChunkReader.java ---
@@ -56,13 +57,14 @@ MeasureRawColumnChunk readRawMeasureChunk(FileReader fileReader, int columnIndex
* @return
* @throws IOException
*/
- ColumnPage decodeColumnPage(MeasureRawColumnChunk measureRawColumnChunk,
- int pageNumber) throws IOException, MemoryException;
+ ColumnPage decodeColumnPage(MeasureRawColumnChunk measureRawColumnChunk, int pageNumber,
+ ReusableDataBuffer reusableDataBuffer) throws IOException, MemoryException;
/**
* Decode raw data and fill the vector
*/
- void decodeColumnPageAndFillVector(MeasureRawColumnChunk measureRawColumnChunk,
- int pageNumber, ColumnVectorInfo vectorInfo) throws IOException, MemoryException;
+ void decodeColumnPageAndFillVector(MeasureRawColumnChunk measureRawColumnChunk, int pageNumber,
+ ColumnVectorInfo vectorInfo, ReusableDataBuffer reusableDataBuffer)
+ throws IOException, MemoryException;
--- End diff --
Can we have separate interface methods with and wothout `ReusableDataBuffer` for both the above methods. Many places null is being passed which can be avoided using separate interface methods and then the common code can be refactored for classes implementing these methods
---
[GitHub] carbondata issue #2872: [WIP] Added reusable buffer code
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2872
Build Success with Spark 2.3.1, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/9726/
---
[GitHub] carbondata issue #2872: [WIP] Added reusable buffer code
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2872
Build Success with Spark 2.3.1, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/9650/
---
[GitHub] carbondata issue #2872: [WIP] Added reusable buffer code
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2872
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1208/
---
[GitHub] carbondata issue #2872: [WIP] Added reusable buffer code
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2872
Build Failed with Spark 2.3.1, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/9656/
---
[GitHub] carbondata issue #2872: [WIP] Added reusable buffer code
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2872
Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1460/
---
[GitHub] carbondata issue #2872: [WIP] Added reusable buffer code
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2872
Build Failed with Spark 2.3.1, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/9472/
---