GitHub user kumarvishal09 opened a pull request:
https://github.com/apache/carbondata/pull/2815 [WIP] ### What changes are proposed in this PR **Method In-lining Optimization** JIT will inline any method if method size is less than 325 byte code size and if it is called more than 10K times(default value). If method is private or static it will be easier for JIT to inline as type safe check is not required, for protected/public method it will add a overhead of type check and because of this it will not behave as inline. Because of above case some refactoring is done for primitive no dictionary data type columns. Earlier ColumnPageWrapper.java was handling query processing for all primitive no dictionary data type column now in This PR separate classes are created for each data type handling and all the HOT method is kept as private and protected methods are overridden and other methods are added in Super classes - [ ] 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 JITBasedOPtimization Alternatively you can review and apply these changes as the patch at: https://github.com/apache/carbondata/pull/2815.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 #2815 ---- commit f7c8c16610dc332b4385d8970c5aac2272b84623 Author: kumarvishal09 <kumarvishal1802@...> Date: 2018-10-15T14:32:40Z PrimitiveDimColumnPage ---- --- |
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2815 Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/793/ --- |
In reply to this post by qiuchenjian-2
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2815 Build Failed with Spark 2.3.1, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/9058/ --- |
In reply to this post by qiuchenjian-2
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2815 Build Failed with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/990/ --- |
In reply to this post by qiuchenjian-2
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2815 Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/800/ --- |
In reply to this post by qiuchenjian-2
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2815 Build Success with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/997/ --- |
In reply to this post by qiuchenjian-2
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2815 Build Success with Spark 2.3.1, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/9065/ --- |
In reply to this post by qiuchenjian-2
Github user kunal642 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2815#discussion_r225779843 --- Diff: core/src/main/java/org/apache/carbondata/core/datastore/chunk/impl/AbstractPrimitiveDimColumnPage.java --- @@ -0,0 +1,110 @@ +/* + * 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.chunk.impl; + +import java.util.BitSet; + +import org.apache.carbondata.core.datastore.chunk.DimensionColumnPage; +import org.apache.carbondata.core.datastore.page.ColumnPage; + +/** + * Abstract Class for handling Primitive data type Dimension column + */ +public abstract class AbstractPrimitiveDimColumnPage implements DimensionColumnPage { + + /** + * column page which holds the actual data + */ + protected ColumnPage columnPage; + + /** + * inverted index store the actual position of the data + */ + protected int[] invertedIndex; + + /** + * reverse index store the position after sorting the data + */ + protected int[] invertedIndexReverse; + + /** + * boolean to check if data was exolictly sorted or not --- End diff -- Please fix the typo exolictly => explicitly --- |
In reply to this post by qiuchenjian-2
Github user kunal642 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2815#discussion_r225779902 --- Diff: core/src/main/java/org/apache/carbondata/core/datastore/chunk/impl/AbstractPrimitiveDimColumnPage.java --- @@ -0,0 +1,110 @@ +/* + * 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.chunk.impl; + +import java.util.BitSet; + +import org.apache.carbondata.core.datastore.chunk.DimensionColumnPage; +import org.apache.carbondata.core.datastore.page.ColumnPage; + +/** + * Abstract Class for handling Primitive data type Dimension column + */ +public abstract class AbstractPrimitiveDimColumnPage implements DimensionColumnPage { + + /** + * column page which holds the actual data + */ + protected ColumnPage columnPage; + + /** + * inverted index store the actual position of the data + */ + protected int[] invertedIndex; + + /** + * reverse index store the position after sorting the data + */ + protected int[] invertedIndexReverse; + + /** + * boolean to check if data was exolictly sorted or not + */ + protected boolean isExplictSorted; + + /** + * null bitset represents cell with null value + */ + protected BitSet nullBitset; + + protected boolean isAllNullValues; + + protected AbstractPrimitiveDimColumnPage(ColumnPage columnPage, int[] invertedIndex, + int[] invertedIndexReverse, int numberOfRows) { + this.columnPage = columnPage; + this.isExplictSorted = null != invertedIndex && invertedIndex.length > 0; + this.invertedIndex = invertedIndex; + this.invertedIndexReverse = invertedIndexReverse; + this.nullBitset = columnPage.getNullBits(); + isAllNullValues = nullBitset.cardinality() == numberOfRows; + } + + @Override public int fillRawData(int rowId, int offset, byte[] data) { + return 0; + } + + @Override public int fillSurrogateKey(int rowId, int chunkIndex, int[] outputSurrogateKey) { + return 0; + } + + @Override public boolean isAdaptiveEncoded() { + return true; + } + + @Override public void freeMemory() { + if (null != columnPage) { + columnPage.freeMemory(); + this.invertedIndexReverse = null; + this.invertedIndex = null; + columnPage = null; + } + } + + @Override public boolean isNoDicitionaryColumn() { --- End diff -- isNoDicitionaryColumn => isNoDictionaryColumn --- |
In reply to this post by qiuchenjian-2
Github user kunal642 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2815#discussion_r225784603 --- Diff: core/src/main/java/org/apache/carbondata/core/datastore/chunk/reader/dimension/v3/CompressedDimensionChunkFileBasedReaderV3.java --- @@ -245,22 +253,43 @@ protected DimensionColumnPage decodeDimension(DimensionRawColumnChunk rawColumnP ColumnPage decodedPage = decodeDimensionByMeta(pageMetadata, pageData, offset, null != rawColumnPage.getLocalDictionary()); decodedPage.setNullBits(QueryUtil.getNullBitSet(pageMetadata.presence, this.compressor)); - int[] invertedIndexes = new int[0]; - int[] invertedIndexesReverse = new int[0]; - // in case of no dictionary measure data types, if it is included in sort columns - // then inverted index to be uncompressed - if (encodings.contains(Encoding.INVERTED_INDEX)) { - offset += pageMetadata.data_page_length; - if (CarbonUtil.hasEncoding(pageMetadata.encoders, Encoding.INVERTED_INDEX)) { - invertedIndexes = CarbonUtil - .getUnCompressColumnIndex(pageMetadata.rowid_page_length, pageData, offset); - // get the reverse index - invertedIndexesReverse = CarbonUtil.getInvertedReverseIndex(invertedIndexes); + ColumnType columnType = decodedPage.getColumnSpec().getColumnType(); + if (ColumnType.COMPLEX_ARRAY == columnType || ColumnType.COMPLEX_STRUCT == columnType + || ColumnType.COMPLEX_PRIMITIVE == columnType || ColumnType.COMPLEX == columnType) { --- End diff -- "ColumnType.COMPLEX == columnType" is enought to check for complex column, "ColumnType.COMPLEX_ARRAY == columnType || ColumnType.COMPLEX_STRUCT == columnType" is not required. --- |
In reply to this post by qiuchenjian-2
Github user xuchuanyin commented on the issue:
https://github.com/apache/carbondata/pull/2815 @kumarvishal09 What about the performance gained after this optimization? --- |
In reply to this post by qiuchenjian-2
Github user kumarvishal09 commented on the issue:
https://github.com/apache/carbondata/pull/2815 Closing this PR as handled as part of #2819 and One more PR will be raised based on #2819 Interface --- |
In reply to this post by qiuchenjian-2
Github user kumarvishal09 closed the pull request at:
https://github.com/apache/carbondata/pull/2815 --- |
Free forum by Nabble | Edit this page |