Will-Lo commented on code in PR #3751: URL: https://github.com/apache/gobblin/pull/3751#discussion_r1310733905
########## gobblin-modules/gobblin-orc/src/main/java/org/apache/gobblin/writer/OrcConverterMemoryManager.java: ########## @@ -0,0 +1,86 @@ +/* + * 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.gobblin.writer; + +import org.apache.orc.storage.ql.exec.vector.ColumnVector; +import org.apache.orc.storage.ql.exec.vector.ListColumnVector; +import org.apache.orc.storage.ql.exec.vector.MapColumnVector; +import org.apache.orc.storage.ql.exec.vector.StructColumnVector; +import org.apache.orc.storage.ql.exec.vector.UnionColumnVector; +import org.apache.orc.storage.ql.exec.vector.VectorizedRowBatch; + + +/** + * A helper class to calculate the size of array buffers in a {@link VectorizedRowBatch}. + * This estimate is mainly based on the maximum size of each variable length column, which can be resized + * Since the resizing algorithm for each column can balloon, this can affect likelihood of OOM + */ +public class OrcConverterMemoryManager { + + private VectorizedRowBatch rowBatch; + + // TODO: Consider moving the resize algorithm from the converter to this class + OrcConverterMemoryManager(VectorizedRowBatch rowBatch) { + this.rowBatch = rowBatch; + } + + /** + * Calculates the maximum number of elements of lists and maps in a column + * @param col + * @return + */ + public long calculateSizeOfColHelper(ColumnVector col) { + long converterBufferColSize = 0; + if (col instanceof ListColumnVector) { + ListColumnVector listColumnVector = (ListColumnVector) col; + converterBufferColSize += listColumnVector.child.isNull.length; Review Comment: 1. It should be the length of the current list, `child` refers to the element within the columnVector. Since this is columnar need to think of the child schema, how many of that child is within this column. 2. ~Ah good point, I guess this will catch scenarios of a list within a list, will modify.~ Originally I thought we had to address (2) but the columnar nature of ORC means that you just need to ensure that only that particular column's child (being the element in the array) can hold all the items in the array's length. So addition is fine here. 3. I wanted to measure the space filled by the resizes, which is primarily represented by an array size increase of a boolean array. I'm not sure if Java measures null in an array with the size of the expected object * length. Edited: Okay after looking into the library implementation most of the primitive ORC types use a null value that maps to a default java primitive type, so there is a size associated which can be roughly estimated. Will use that as the benchmark and it should lead to improvements in accuracy -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
