Excessive memory allocation for primitive arrays in nested resultmaps #927

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
3 participants
@simplyarjen

When mapping a result using a resultmap that contains a byte[] property and an <association> mapping, mybatis allocates a Byte object for each byte in the array during the mapping process. This leads to several gigabytes of memory being allocated for the mapping of a 100 megabyte array, which makes the difference between a memory-intensive task and an OutOfMemoryError.

See this repository for a reproduction. The root of the allocation is the CacheKey built up in DefaultResultSetHandler#handleRowValuesForNestedResultMap. This logic in CacheKey#update results in a boxed object being allocated for each byte in the array value.

This logic in CacheKey was introduced in this commit to solve #124 and #125. Arrays use object identity rather than considering they contents for equals and hashCode computations. For cache keys, the later is obviously more desirable, and this is achieved by adding the elements of the array to the updateList, rather than the array itself. Unfortunately, java.lang.reflect.Array#get always allocates a new boxed primitive, circumventing e.g. the caching in java.lang.Byte#valueOf which is used in 'normal' boxing.

The attached commit achieves the content based equals and hashCode by adding a wrapper object around the array, which delegates to the appropriate functions in java.util.Arrays that consider the contents of the arrays as required.

@harawata

This comment has been minimized.

Show comment
Hide comment
@harawata

harawata Feb 27, 2017

Member

Thank you, @simplyarjen . I'll look into it.

Member

harawata commented Feb 27, 2017

Thank you, @simplyarjen . I'll look into it.

@harawata harawata closed this in 80de131 Feb 28, 2017

@harawata

This comment has been minimized.

Show comment
Hide comment
@harawata

harawata Feb 28, 2017

Member

Hi @simplyarjen ,

Thank you for the report and the demo app!

I have fixed it slightly differently (uses java.util.Arrays methods, but no wrappers).
The fix is tested with your demo app.
Please let me know in case I am missing something.

Note that cause of this issue is one of the disadvantages brought by omitting <id /> in result map which is mentioned in the doc.

Even without this fix, the demo app runs fine if id elements are added to the result map.

<resultMap id="FileMap" type="co.rngd.mybatis.File">
  <id column="id" />
  <result property="content" column="data" />
  <association property="metadata" resultMap="MetadataMap" />
</resultMap>

<resultMap id="MetadataMap" type="co.rngd.mybatis.Metadata">
  <id column="id" />
  <result property="name" column="name" />
  <result property="sizeInBytes" column="size" />
</resultMap>

As the target objects do not have 'id' fields, there is no 'property' attribute in <id />.
MyBatis still uses them when building cache keys, so it's more efficient.

Member

harawata commented Feb 28, 2017

Hi @simplyarjen ,

Thank you for the report and the demo app!

I have fixed it slightly differently (uses java.util.Arrays methods, but no wrappers).
The fix is tested with your demo app.
Please let me know in case I am missing something.

Note that cause of this issue is one of the disadvantages brought by omitting <id /> in result map which is mentioned in the doc.

Even without this fix, the demo app runs fine if id elements are added to the result map.

<resultMap id="FileMap" type="co.rngd.mybatis.File">
  <id column="id" />
  <result property="content" column="data" />
  <association property="metadata" resultMap="MetadataMap" />
</resultMap>

<resultMap id="MetadataMap" type="co.rngd.mybatis.Metadata">
  <id column="id" />
  <result property="name" column="name" />
  <result property="sizeInBytes" column="size" />
</resultMap>

As the target objects do not have 'id' fields, there is no 'property' attribute in <id />.
MyBatis still uses them when building cache keys, so it's more efficient.

@kazuki43zoo kazuki43zoo added this to the 3.4.3 milestone Feb 28, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment