ResultSetHandler need performance optimization

[GoogleCodeExporter]

What version of the MyBatis are you using?
3.01 ga

Please describe the problem.  Unit tests are best!

FastResultSetHandler and NestedResultSetHandler need performance optimization.

handleResultSet() call handleRowValues(), then handleRowValues() call 
handleRowValue(), the problem is each handleRowValue() call 
loadMappedAndUnmappedColumnNames() generate mappedColumnNames and 
unMappedColumnNames, especially if there is no resultmap, the 
applyAutomaticMappings() method call metaObject.findProperty() and 
typeHandlerRegistry.getTypeHandler() for each column of the row when use auto 
mapping. 

So there has performance problem if the select statement has many column and 
has many rows. 

I think this design mode mainly resolve Discriminator dynamic resultmapping, 
but many case there is no resultmap, even if exists resultmap, but many case 
there is rarely use Discriminator. 

What is the expected output? What do you see instead?

I modify the related method, see the attachment

Please provide any additional information below.

Original issue reported on code.google.com by myweboff...@gmail.com on 28 Jun 2010 at 9:45

Attachments:

• FastResultSetHandler.java
• NestedResultSetHandler.java

[GoogleCodeExporter]

in huge resultsets this completely breaks the utility of using mybatis. i don't 
understand its tagged as low priority

Original comment by asaru...@gmail.com on 24 Sep 2010 at 1:33

[GoogleCodeExporter]

a query result is just 5MB, but objects maded in handlers is over 200MB. a 
enden memory is dancing now in my jconsole graph. :(
i'm sure this issue need more attention.

Original comment by passion....@gmail.com on 29 Nov 2011 at 5:04

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

could you please provide this change as a patch?

Original comment by eduardo.macarron on 29 Nov 2011 at 7:03

[GoogleCodeExporter]

Hi,

I have attached an experimental patch that should improve the performance in 
particular cases.
It basically reduces the time of type handler resolution for each column.
I also tried to optimize the other methods you mentioned, but they had smaller 
impact on my tests.

Please try the patch and let us know the result.
And if the patch does not improve the performance, please attach a test case or 
sample files so that we can reproduce the problem (no need to include all rows).

Thanks,
Iwao

Original comment by haraw...@gmail.com on 18 Jan 2012 at 5:23

Attachments:

• issue59.patch

[GoogleCodeExporter]

Hi Iwao,

I've pushed your change a bit further. This one caches also the mapped/unmapped 
columns as the original author of this issue suggested.

That should also improve performance a bit, mostly on big results.

Original comment by eduardo.macarron on 21 Jan 2012 at 9:34

[GoogleCodeExporter]

Original comment by eduardo.macarron on 21 Jan 2012 at 9:35

Attachments:

• issue59-2.patch

[GoogleCodeExporter]

Thanks Eduardo for nice additions!

A few minor proposals:
* the internal class could be declared as protected.
* loadMappedAndUnmappedColumnNames() method could use columnNames field value 
instead of ResultSetMetaData.
Please see the attached patch.

And I definitely picked a wrong name for the class (it does not wrap 
ResultSetMetaData at all).
How about ResultColumnCache or ResultColumnMetaData?

Original comment by haraw...@gmail.com on 21 Jan 2012 at 2:18

Attachments:

• issue59-2.1.patch

[GoogleCodeExporter]

Agreed with the changes.

I am not sure I do fully understand the change in type handlers. I would say 
that in terms of performance the new version looks also in a hashmap like the 
original version. But it tries with java class got or jdbc from column 
introspection if there is no handler for the property. On which cases do this 
suppose a benefit?

Thanks in advance!

Original comment by eduardo.macarron on 21 Jan 2012 at 5:19

[GoogleCodeExporter]

Hi Eduardo,

That is the part I called 'experimental' ;)
It is actually copied from UnknownTypeHandler so that it does not break any 
backward-compatibility.

In short, when auto-mapping is enabled and "map" is used as resultType, MyBatis 
ended up calling  UnknownTypeHandler#resolveTypeHandler() which does the 
similar thing.
The attached is the test case I used (and visualvm CPU profile result against 
r4588 in case you are interested).

Hope this answers your question, but if it doesn't, please let me know.

Thank you,
Iwao

Original comment by haraw...@gmail.com on 21 Jan 2012 at 7:05

Attachments:

• issue59.zip
• map_cpu.nps

[GoogleCodeExporter]

Ok. That is what I was missing, the code inside UnknownTypeHandler.

Profiler data is surpising, 26% of CPU time is inside UnknownTypeHandler. My 
laptop runs your sample in r4588 in 3,5 segs and your patch reduces that time 
to 2,4 seg. And mine just a 0,2 seconds more. 

BTW I was a bit afraid because I thought that was a change. Now I am much more 
happy ;) 

So +1 to commit it.

Awesome work again Iwao!

Original comment by eduardo.macarron on 21 Jan 2012 at 11:49

[GoogleCodeExporter]

Thanks for a detailed review!
I would appreciate any question or advise :)

I will commit the patch later, then.
Not so sure if it fixes the reported issue, but the performance gain may be 
worth a commit.

Thank you,
Iwao

Original comment by haraw...@gmail.com on 23 Jan 2012 at 12:33

[GoogleCodeExporter]

I forgot to ask you about the TODO comment in 
FastResultSetHandler#handleRefCursorOutputParameter.
Should it be retained in the commit?

Original comment by haraw...@gmail.com on 23 Jan 2012 at 1:57

[GoogleCodeExporter]

No Iwao, I put that while I was starting the change and forgot to remove it.

Original comment by eduardo.macarron on 23 Jan 2012 at 4:01

[GoogleCodeExporter]

I have just seen you committed the change in r4599. I suppose we can close the 
issue. 

Thank you!!

Original comment by eduardo.macarron on 23 Jan 2012 at 7:30

• Changed state: Fixed
• Added labels: Target-Release3.1.0, Component-SqlMaps

[GoogleCodeExporter]

I wasn't sure if I should close this issue, thank you!

Original comment by haraw...@gmail.com on 24 Jan 2012 at 3:48

[GoogleCodeExporter]

Original comment by eduardo.macarron on 5 Feb 2012 at 6:56

• Added labels: Version-Release3.x