Updates to PR-98 #2

isnotinvain · 2015-02-21T04:34:36Z

Use simpler way to calculate starting size for CapacityByteArrayOutputStream + use smaller starting sizes
Don't use CapacityByteArrayOutputStream for writing page chunks

… a simpler heuristic in the column writers instead

isnotinvain · 2015-02-21T04:36:39Z

parquet-column/src/main/java/parquet/column/values/dictionary/DictionaryValuesWriter.java

@@ -143,7 +143,7 @@ public BytesInput getBytes() {
    if (DEBUG) LOG.debug("max dic id " + maxDicId);
    int bitWidth = BytesUtils.getWidthFromMaxInt(maxDicId);
    // TODO: what is a good initialCapacity?
-    RunLengthBitPackingHybridEncoder encoder = new RunLengthBitPackingHybridEncoder(bitWidth, 64 * 1024, maxDictionaryByteSize);
+    RunLengthBitPackingHybridEncoder encoder = new RunLengthBitPackingHybridEncoder(bitWidth, 64, maxDictionaryByteSize);


Let me know if this is a bad idea, this was tripping the check I added for initialSize must be < maxSizeHint

should this be the following instead of 64?
max(MIN_SLAB_SIZE, ((int) (maxDictionaryByteSize / pow(2, 10))))
which makes me think this should probably handled inside this based on maxDictionaryByteSize

Well I'm not sure what's going on here actually.
This is the value column for dictionary encoded values right?
So yeah, we could do the same thing where we aim for doubling 10 times, but... aren't we hoping that this RLE column will be tiny in many cases? or is maxDictionaryByteSize already pretty small?

julienledem · 2015-02-23T20:26:22Z

parquet-encoding/src/main/java/parquet/bytes/CapacityByteArrayOutputStream.java

  }

  @Override
  public void write(byte b[], int off, int len) {
    if ((off < 0) || (off > b.length) || (len < 0) ||
        ((off + len) - b.length > 0)) {
-      throw new IndexOutOfBoundsException();
+      throw new IndexOutOfBoundsException(String.format("len: %d, off: %d", len, off));


…onaryValuesWriter as well

isnotinvain · 2015-02-26T02:06:03Z

I ran a test and tried writing 1.8GB of lzo-thrift (6.2GB w/o compression) to parquet on a schema with many columns, using a heap size of 3GB. On parquet master this almost immediately crashes w/ OOM. On this branch, it ran to completion in about 25 minutes. (the output size was slightly larger than 1.8GB, though could be due to bad tuning).

isnotinvain · 2015-02-26T02:06:46Z

Also: I was not able to get the native lzo bindings to work on the machine I ran this test on, I don't know if that would have any impact on memory usage but I imagine it could only make it worse right?

julienledem · 2015-02-26T20:18:41Z

parquet-hadoop/src/main/java/parquet/hadoop/ColumnChunkPageWriteStore.java

-      compressedData.writeAllTo(buf);
+      buf.collect(repetitionLevels);
+      buf.collect(definitionLevels);
+      buf.collect(compressedData);


you can just make one collect with all the combined BytesInput into one logically concatenated one

isnotinvain · 2015-02-27T21:19:27Z

OK I think this is ready to merge, then we can do a final pass if needed on PR 98

Updates to PR-98

…2 api Currently for creating a user defined predicate using the new filter api, no value can be passed to create a dynamic filter at runtime. This reduces the usefulness of the user defined predicate, and meaningful predicates cannot be created. We can add a generic Object value that is passed through the api, which can internally be used in the keep function of the user defined predicate for creating many different types of filters. For example, in spark sql, we can pass in a list of filter values for a where IN clause query and filter the row values based on that list. Author: Yash Datta <[email protected]> Author: Alex Levenson <[email protected]> Author: Yash Datta <[email protected]> Closes apache#73 from saucam/master and squashes the following commits: 7231a3b [Yash Datta] Merge pull request #3 from isnotinvain/alexlevenson/fix-binary-compat dcc276b [Alex Levenson] Ignore binary incompatibility in private filter2 class 7bfa5ad [Yash Datta] Merge pull request #2 from isnotinvain/alexlevenson/simplify-udp-state 0187376 [Alex Levenson] Resolve merge conflicts 25aa716 [Alex Levenson] Simplify user defined predicates with state 51952f8 [Yash Datta] PARQUET-116: Fix whitespace d7b7159 [Yash Datta] PARQUET-116: Make UserDefined abstract, add two subclasses, one accepting udp class, other accepting serializable udp instance 40d394a [Yash Datta] PARQUET-116: Fix whitespace 9a63611 [Yash Datta] PARQUET-116: Fix whitespace 7caa4dc [Yash Datta] PARQUET-116: Add ConfiguredUserDefined that takes a serialiazble udp directly 0eaabf4 [Yash Datta] PARQUET-116: Move the config object from keep method to a configure method in udp predicate f51a431 [Yash Datta] PARQUET-116: Adding type safety for the filter object to be passed to user defined predicate d5a2b9e [Yash Datta] PARQUET-116: Enforce that the filter object to be passed must be Serializable dfd0478 [Yash Datta] PARQUET-116: Add a test case for passing a filter object to user defined predicate 4ab46ec [Yash Datta] PARQUET-116: Pass a filter object to user defined predicate in filter2 api

This buffer initializes itself to a default size when instantiated. This leads to a lot of unused small buffers when there are a lot of empty columns. Author: Alex Levenson <[email protected]> Author: julien <[email protected]> Author: Julien Le Dem <[email protected]> Closes apache#98 from julienledem/avoid_wasting_64K_per_empty_buffer and squashes the following commits: b0200dd [julien] add license a1b278e [julien] Merge branch 'master' into avoid_wasting_64K_per_empty_buffer 5304ee1 [julien] remove unused constant 81e399f [julien] Merge branch 'avoid_wasting_64K_per_empty_buffer' of github.com:julienledem/incubator-parquet-mr into avoid_wasting_64K_per_empty_buffer ccf677d [julien] Merge branch 'master' into avoid_wasting_64K_per_empty_buffer 37148d6 [Julien Le Dem] Merge pull request #2 from isnotinvain/PR-98 b9abab0 [Alex Levenson] Address Julien's comment 965af7f [Alex Levenson] one more typo 9939d8d [Alex Levenson] fix typos in comments 61c0100 [Alex Levenson] Make initial slab size heuristic into a helper method, apply in DictionaryValuesWriter as well a257ee4 [Alex Levenson] Improve IndexOutOfBoundsException message 64d6c7f [Alex Levenson] update comments 8b54667 [Alex Levenson] Don't use CapacityByteArrayOutputStream for writing page chunks 6a20e8b [Alex Levenson] Remove initialSlabSize decision from InternalParquetRecordReader, use a simpler heuristic in the column writers instead 3a0f8e4 [Alex Levenson] Use simpler settings for column chunk writer b2736a1 [Alex Levenson] Some cleanup in CapacityByteArrayOutputStream 1df4a71 [julien] refactor CapacityByteArray to be aware of page size 95c8fb6 [julien] avoid wasting 64K per empty buffer.

isnotinvain added 4 commits February 20, 2015 16:37

Some cleanup in CapacityByteArrayOutputStream

b2736a1

Use simpler settings for column chunk writer

3a0f8e4

Remove initialSlabSize decision from InternalParquetRecordReader, use…

6a20e8b

… a simpler heuristic in the column writers instead

Don't use CapacityByteArrayOutputStream for writing page chunks

8b54667

isnotinvain reviewed Feb 21, 2015
View reviewed changes

isnotinvain mentioned this pull request Feb 21, 2015

PARQUET-160: avoid wasting 64K per empty buffer. apache/parquet-java#98

Closed

update comments

64d6c7f

julienledem reviewed Feb 23, 2015
View reviewed changes

isnotinvain added 4 commits February 23, 2015 13:37

Improve IndexOutOfBoundsException message

a257ee4

Make initial slab size heuristic into a helper method, apply in Dicti…

61c0100

…onaryValuesWriter as well

fix typos in comments

9939d8d

one more typo

965af7f

julienledem reviewed Feb 26, 2015
View reviewed changes

Address Julien's comment

b9abab0

julienledem added a commit that referenced this pull request Feb 28, 2015

Merge pull request #2 from isnotinvain/PR-98

37148d6

Updates to PR-98

julienledem merged commit 37148d6 into julienledem:avoid_wasting_64K_per_empty_buffer Feb 28, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Updates to PR-98 #2

Updates to PR-98 #2

isnotinvain commented Feb 21, 2015

isnotinvain Feb 21, 2015

julienledem Feb 23, 2015

isnotinvain Feb 23, 2015

julienledem Feb 23, 2015

isnotinvain Feb 23, 2015

isnotinvain commented Feb 26, 2015

isnotinvain commented Feb 26, 2015

julienledem Feb 26, 2015

isnotinvain Feb 27, 2015

isnotinvain commented Feb 27, 2015

Updates to PR-98 #2

Updates to PR-98 #2

Conversation

isnotinvain commented Feb 21, 2015

isnotinvain Feb 21, 2015

Choose a reason for hiding this comment

julienledem Feb 23, 2015

Choose a reason for hiding this comment

isnotinvain Feb 23, 2015

Choose a reason for hiding this comment

julienledem Feb 23, 2015

Choose a reason for hiding this comment

isnotinvain Feb 23, 2015

Choose a reason for hiding this comment

isnotinvain commented Feb 26, 2015

isnotinvain commented Feb 26, 2015

julienledem Feb 26, 2015

Choose a reason for hiding this comment

isnotinvain Feb 27, 2015

Choose a reason for hiding this comment

isnotinvain commented Feb 27, 2015