Hitesh, I suggest you file a JIRA for the potential issue you are seeing and if possible raise a PR with a test case that you think is broken with current code. Happy to discuss on Jira or PR.
Thanks, Siddharth On Thu, Mar 28, 2019 at 11:20 AM Hitesh <hitesh...@yahoo.com> wrote: > Hi Siddharth: > > Here, I see a problem in line#162, where its taking "bufferSize" to find > the extra allocated bytes. It should be "valueCount*typeWidth + > valueCount/8". > > Here is an example for that. Let's take 1000 ints. Then, > valueCount = 1000 ints > typWidth = 4 bytes > validitiyBufferSize = 125 bytes > valueBufferSize = 4000 bytes > combinedSize(valueBufferSize + validityBufferSize) = 4128 bytes (multiple > of 8) > combinedSizeWith2ThePowerSize = 8192 bytes, this will be "bufferSize" at > line#152. > > With the above calculation, this code should release > (combinedSizeWith2ThePowerSize - combinedSize) = 4064 bytes. But, this is > not happening. > > let me know if this example helps. Do we have some other channel to talk? > > Thanks. > Hitesh. > > > > > > > On Thursday, March 28, 2019, 10:59:18 AM PDT, Siddharth Teotia < > siddha...@dremio.com> wrote: > > > > > > Hitesh, > > Yes, if you see in the code, the sliced buffers have their reference > counts bumped up before the compound buffer is released. Bumping up the > reference counts of child/sliced buffers allows us to release the compound > buffer safely. Does that make sense? > > Thanks, > Siddharth > > On Wed, Mar 27, 2019 at 12:45 PM Hitesh <hitesh...@yahoo.com.invalid> > wrote: > > Hi Siddarth: > > Thanks. yes, I am referring compound buffer as an extra buffer. This we > release and further can be reused? > > Let's take an example of 1000 ints. > > Then, it will need the following bytes. > > getValidityBufferSize: 125value bufferSize: 4000combinedSize: > 4128combinedSizeWith2ThePower: 8192 > > Then, that code should release (combinedSizeWith2ThePower - CombinedSize > = 4064) bytes? I think thats the intention of that code but its considering > other calculated value. > > Please let me know what you think about it? > > Thanks.Hitesh. > > On Wednesday, March 27, 2019, 12:23:47 PM PDT, Siddharth Teotia < > siddha...@dremio.com> wrote: > > > > Hi Hitesh, > > > > The code you referenced allocates data and validity buffers for a fixed > > width vector. It first determines the appropriate buffer size for a given > > value count and then allocates a compound buffer. The compound buffer is > > then sliced to get data and validity buffers and finally compound buffer > is > > released. Were you referring to compound buffer as extra buffer? > > > > Also, actualCount can't be equal to valueCount * typeWidth since it > > represents the number of values that can be stored in the vector. > > valueCount * typeWidth will give you the buffer size to be allocated for > a > > certain value count and data type. > > > > Thanks, > > Siddharth > > > > On Wed, Mar 27, 2019 at 11:58 AM Hitesh Khamesra > > <hitesh...@yahoo.com.invalid> wrote: > > > >> Hi All: > >> I was looking following code to release extra allocated buffer. It seems > >> it should be considering actualCount as "valueCount*typeWidth". Then it > >> should calculate extra buffer and release it. Right now, it calculates > >> based on actually allocated size and not justifying the intend. Any > >> thought?? > >> ===============================================line 162 at " > >> > https://github.com/apache/arrow/blob/master/java/vector/src/main/java/org/apache/arrow/vector/BaseValueVector.java > >> " > >> protected DataAndValidityBuffers allocFixedDataAndValidityBufs(int > >> valueCount, int typeWidth) { > >> long bufferSize = computeCombinedBufferSize(valueCount, typeWidth); > >> assert bufferSize < MAX_ALLOCATION_SIZE; > >> > >> int validityBufferSize; > >> int dataBufferSize; > >> if (typeWidth == 0) { > >> validityBufferSize = dataBufferSize = (int) (bufferSize / 2); > >> } else { > >> // Due to roundup to power-of-2 allocation, the bufferSize could be > >> greater than the > >> // requested size. Utilize the allocated buffer fully.; > >> int actualCount = (int) ((bufferSize * 8.0) / (8 * typeWidth + 1)); > >> do { > >> validityBufferSize = (int) > >> roundUp8(getValidityBufferSizeFromCount(actualCount)); > >> dataBufferSize = (int) roundUp8(actualCount * typeWidth); > >> if (validityBufferSize + dataBufferSize <= bufferSize) { > >> break; > >> } > >> --actualCount; > >> } while (true); > >> } > >> ==================================================== > >> Thanks.Hitesh. > >> > > >
