Skip to content

Conversation

Oliverwqcwrw
Copy link
Contributor

No description provided.

@coveralls
Copy link

coveralls commented Apr 26, 2022

Coverage Status

Coverage remained the same at 87.299% when pulling 1a9e632 on Oliverwqcwrw:master-improve-performance-8 into a68ba4b on mybatis:master.

@harawata
Copy link
Member

Hello @Oliverwqcwrw ,

Thank you, but the effect of this change may be minimal.
If you have data that proves otherwise, please post it along with the code that was used to measure the difference.

@Oliverwqcwrw
Copy link
Contributor Author

Oliverwqcwrw commented Apr 29, 2022

My idea is initialized by size to prevent namespaceDiscriminatorMap expansion, because the discriminator case node more than a dozen will trigger the expansion
e.g.

   <discriminator javaType="String" column="did">
        <case value="101" resultType="empInfo">
            <result column="ename" property="name"/>
        </case>
        <case value="102" resultType="empInfo">
            <result column="ejob" property="job"/>
        </case>
        <case value="103" resultType="empInfo">
            <result column="eage" property="age"/>
        </case>
        <case value="104" resultType="empInfo">
            <result column="ejob" property="job"/>
        </case>
        <case value="105" resultType="empInfo">
            <result column="ephone" property="phone"/>
        </case>
        <case value="106" resultType="empInfo">
            <result column="ejob" property="job"/>
        </case>
        <case value="107" resultType="empInfo">
            <result column="ename" property="name"/>
        </case>
        <case value="108" resultType="empInfo">
            <result column="ejob" property="job"/>
        </case>
        <case value="109" resultType="empInfo">
            <result column="eage" property="age"/>
        </case>
        <case value="110" resultType="empInfo">
            <result column="ejob" property="job"/>
        </case>
        <case value="111" resultType="empInfo">
            <result column="ephone" property="phone"/>
        </case>
        <case value="112" resultType="empInfo">
            <result column="ejob" property="job"/>
        </case>
        <case value="113" resultType="empInfo">
            <result column="ename" property="name"/>
        </case>
        <case value="114" resultType="empInfo">
            <result column="ejob" property="job"/>
        </case>
        <case value="115" resultType="empInfo">
            <result column="eage" property="age"/>
        </case>
        <case value="116" resultType="empInfo">
            <result column="ejob" property="job"/>
        </case>
        <case value="117" resultType="empInfo">
            <result column="ephone" property="phone"/>
        </case>
        <case value="118" resultType="empInfo">
            <result column="ejob" property="job"/>
        </case>
    </discriminator>

@harawata
Copy link
Member

I understand your intent, but building mapper occurs only once during application startup and saving a few extra expansion does not make much difference in reality, I think.
As I wrote, if you have data that shows the effect of this change, I would be happy to reconsider.

@Oliverwqcwrw
Copy link
Contributor Author

I have tested the situation of expansion and no expansion, and the data is as follows

Expansion takes nanoTime = 83521
Without expansion takes nanoTime = 8184

Just as the data, the difference is quite big, although the impact of one expansion is relatively small, but in reality, we may have hundreds or even more mapper for business development, if each one saves some expansion time, then the growth of our startup speed will be obvious

@harawata
Copy link
Member

Could you share the project you used to measure the time?

@Oliverwqcwrw
Copy link
Contributor Author

@Test
public void test(){
    HashMap<String, String> map = new HashMap<>();
    final long start1 = System.nanoTime();
    for (int i = 0; i < 20; i++) {
        map.put("key" + i, "value");
    }
    System.out.println("Expansion takes nanoTime = " + (System.nanoTime() - start1));

    HashMap<String,String> map2 = new HashMap<>(20, 1);
    final long start2 = System.nanoTime();
    for (int i = 0; i < 20; i++) {
        map2.put("key" + i, "value");
    }
    System.out.println("Without expansion takes nanoTime = " + (System.nanoTime() - start2));
}

@harawata
Copy link
Member

That really is not the right way to measure performance and the result should be rather random.

So, you are not actually experiencing the performance issue with your project, is that correct?
I appreciate you taking the time to submit a PR, but this type of micro-optimization should be done only when there is an actual problem.
There are some common advices on this question:
https://softwareengineering.stackexchange.com/q/99445

In this particular case, if there are so many discriminators in a result map that map expansion is triggered repeatedly, there may be some kind of design problem and you'll experience real performance issue when mapping the result, I think.

@Oliverwqcwrw
Copy link
Contributor Author

Thanks for your reply,I agree with you

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

Successfully merging this pull request may close these issues.

3 participants