-
Notifications
You must be signed in to change notification settings - Fork 829
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
optimize unnecessary require operation in write_ascii_slow #835
base: master
Are you sure you want to change the base?
Conversation
2d12505
to
a8bc025
Compare
@pyb1993: I have not forgotten about this PR. I'm currently on vacation and will take a look when I'm back in 2 weeks. |
@pyb1993: I just looked into your PR and can reproduce the performance gains. However, I'm not sure about applying your changes, since they only affect a rare corner case. As you wrote on the mailing list:
If performance is important, the output should be pooled and re-used. Otherwise you will see a lot of allocation and a lot of GC pressure. Your change is simple enough, but it still makes the code slightly harder to understand and I'm not sure if any real user would benefit from the it. |
Hi,@theigl I know this is really a rare case,but I found some other case that this function “maxallowedRequired” may be used,such as the writeInts method,which directly use capcacity to compare a threshold。
maybe there are more exra case that can benefit from this function。this function
should be a general function to be used in such situation。
…------------------ Original ------------------
From: Thomas Heigl ***@***.***>
Date: Mon,Jul 5,2021 7:10 PM
To: EsotericSoftware/kryo ***@***.***>
Cc: .... ***@***.***>, Mention ***@***.***>
Subject: Re: [EsotericSoftware/kryo] optimize unnecessary require operation in write_ascii_slow (#835)
@pyb1993: I just looked into your PR and can reproduce the performance gains. However, I'm not sure about applying your changes, since they only affect a rare corner case. As you wrote on the mailing list:
But If there is such a case(and I do a benchmark for this rare case), the performance will have huge improvement in writeAscii.
the length of string is larger than the initial output buffer size. (which will cause the unecessary require operation(allocate and copy))
the output is not reused, new output each time (I think the best practice is reused the output if possible)
If performance is important, the output should be pooled and re-used. Otherwise you will see a lot of allocation and a lot of GC pressure. Your change is simple enough, but it still makes the code slightly harder to understand and I'm not sure if any real user would benefit from the it.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
Hi, how do you think about his PR? as what I talked above, I think there are a lot similar situation that |
@pyb1993: I'm still +0 on this. It can improve things for edge cases, but I'm not sure the change is worth it. I'll try to get some more feedback on this. |
you can see the discussion here
This PR is to reduce the require operation when serialize huge ascii string
You can use