You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by Violeta Georgieva <vi...@apache.org> on 2016/08/24 13:44:14 UTC
Introduce methods read/write with ByteBuffer in CoyoteInputStream/CoyoteOutputStream
Hi,
Currently it is not possible to use
ServletInputStream.read/ServletOutputStream.write methods with ByteBuffer.
To enable such scenarios in Tomcat I would like to extend the Tomcat’s
internal implementation (CoyoteInputStream/CoyoteOutputStream). I already
did some refactoring in ByteChunk/CharChunk/SocketWrapperBase so that I can
introduce read/write with ByteBuffer. I executed Autobahn test suite to
check the performance impact after the refactoring and it showed no
performance regression. If there are no objections I want to commit these
changes in Tomcat 9 and Tomcat 8.5.x.
Best Regards,
Violeta
Re: Introduce methods read/write with ByteBuffer in CoyoteInputStream/CoyoteOutputStream
Posted by Violeta Georgieva <mi...@gmail.com>.
Hi,
2016-09-14 14:37 GMT+03:00 Mark Thomas <ma...@apache.org>:
>
> On 13/09/2016 19:07, Violeta Georgieva wrote:
> > 2016-08-30 15:35 GMT+03:00 Violeta Georgieva <mi...@gmail.com>:
>
> <snip/>
>
> >> I introduced CoyoteOutputStream.write(ByteBuffer) it uses new methods
> >> with ByteBuffer instead of ByteChunk.
> >> Next step is to replace ByteChunk/CharChunk usage in CoyoteOutputStream
> >> with ByteBuffer/CharBuffer thus I will switch to the new methods and
all
> >> CoyoteOutputStream.write method will use them.
> >>
> >
> > I would like to back port these changes to Tomcat 8.5 if you do not have
> > any concerns.
>
> +0.5
>
> I'm a little concerned about destabilising 8.5.x but the CI system looks
> to be doing a good job of catching problems.
In addition to the Tomcat test suite I executed many combinations of
performance tests
and I do not experience any stability or performance problems with the new
code.
> Generally, I like the direction this is heading in. Reducing copying
> should improve performance but the impact of that doesn't seem to be
> noticeable so far. I do wonder why. Time to add some performance testing
> to my TODO list I think.
Yes I reduced the copy operations, but there is still copy operation when
the socket write buffer is not empty.
> Currently, there are essentially two code paths for read and write.
>
> 1. Original. User facing API uses byte[]. byte[] retained all the way to
> the SocketWrapper where it is copied to ByteBuffer.
The byte[] is wrapped to a ByteBuffer in order to use the new API and when
possible copy is skipped.
org.apache.catalina.connector.OutputBuffer is working with ByteBuffer after
my changes.
> 2. New. User facing API uses ByteBuffer. ByteBuffer retained all the way
> to to the SocketWrapper where it is used if possible or copied to
> another ByteBuffer if not.
>
> I haven't explored this at all so far, but my initial impression is that
> a lot of duplicated functionality could be removed if the original code
> path copied to a ByteBuffer earlier.
Yes my next step is to deprecate and remove the duplicated functionality as
it is not used as I mentioned above.
Regards,
Violeta
> Mark
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
Re: Introduce methods read/write with ByteBuffer in
CoyoteInputStream/CoyoteOutputStream
Posted by Mark Thomas <ma...@apache.org>.
On 13/09/2016 19:07, Violeta Georgieva wrote:
> 2016-08-30 15:35 GMT+03:00 Violeta Georgieva <mi...@gmail.com>:
<snip/>
>> I introduced CoyoteOutputStream.write(ByteBuffer) it uses new methods
>> with ByteBuffer instead of ByteChunk.
>> Next step is to replace ByteChunk/CharChunk usage in CoyoteOutputStream
>> with ByteBuffer/CharBuffer thus I will switch to the new methods and all
>> CoyoteOutputStream.write method will use them.
>>
>
> I would like to back port these changes to Tomcat 8.5 if you do not have
> any concerns.
+0.5
I'm a little concerned about destabilising 8.5.x but the CI system looks
to be doing a good job of catching problems.
Generally, I like the direction this is heading in. Reducing copying
should improve performance but the impact of that doesn't seem to be
noticeable so far. I do wonder why. Time to add some performance testing
to my TODO list I think.
Currently, there are essentially two code paths for read and write.
1. Original. User facing API uses byte[]. byte[] retained all the way to
the SocketWrapper where it is copied to ByteBuffer.
2. New. User facing API uses ByteBuffer. ByteBuffer retained all the way
to to the SocketWrapper where it is used if possible or copied to
another ByteBuffer if not.
I haven't explored this at all so far, but my initial impression is that
a lot of duplicated functionality could be removed if the original code
path copied to a ByteBuffer earlier.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: Introduce methods read/write with ByteBuffer in CoyoteInputStream/CoyoteOutputStream
Posted by Violeta Georgieva <mi...@gmail.com>.
2016-10-27 15:42 GMT+03:00 Mark Thomas <ma...@apache.org>:
>
> On 27/10/2016 12:59, Rémy Maucherat wrote:
> > 2016-10-27 13:24 GMT+02:00 Violeta Georgieva <mi...@gmail.com>:
> >
> >> So are you OK to have in Tomcat 8.5 the read/write with ByteBuffer in
> >> CoyoteInputStream/CoyoteOutputStream?
> >> The main purpose for this refactoring was to introduce them without
> >> regression in the performance.
> >> I would like to have these APIs in Tomcat 8.5.
> >>
> >> Ok, as long as you agree about possible further changes to it :) Also,
> > Mark went silent on the topic, so we don't know if he likes it.
>
> I'm neutral.
>
> I've been doing a little performance testing with HTTP/2 and trunk any
> be far the largest bottleneck still appears to be reading and writing to
> the network so at this point I have no concerns from a performance PoV.
Thanks to both of you. I’m going to do the back port as I would like to
have this in the next Tomcat 8.5 release.
Regards,
Violeta
> Mark
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
Re: Introduce methods read/write with ByteBuffer in
CoyoteInputStream/CoyoteOutputStream
Posted by Mark Thomas <ma...@apache.org>.
On 27/10/2016 12:59, R�my Maucherat wrote:
> 2016-10-27 13:24 GMT+02:00 Violeta Georgieva <mi...@gmail.com>:
>
>> So are you OK to have in Tomcat 8.5 the read/write with ByteBuffer in
>> CoyoteInputStream/CoyoteOutputStream?
>> The main purpose for this refactoring was to introduce them without
>> regression in the performance.
>> I would like to have these APIs in Tomcat 8.5.
>>
>> Ok, as long as you agree about possible further changes to it :) Also,
> Mark went silent on the topic, so we don't know if he likes it.
I'm neutral.
I've been doing a little performance testing with HTTP/2 and trunk any
be far the largest bottleneck still appears to be reading and writing to
the network so at this point I have no concerns from a performance PoV.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: Introduce methods read/write with ByteBuffer in CoyoteInputStream/CoyoteOutputStream
Posted by Violeta Georgieva <mi...@gmail.com>.
2016-10-27 14:59 GMT+03:00 Rémy Maucherat <re...@apache.org>:
>
> 2016-10-27 13:24 GMT+02:00 Violeta Georgieva <mi...@gmail.com>:
>
> > So are you OK to have in Tomcat 8.5 the read/write with ByteBuffer in
> > CoyoteInputStream/CoyoteOutputStream?
> > The main purpose for this refactoring was to introduce them without
> > regression in the performance.
> > I would like to have these APIs in Tomcat 8.5.
> >
> > Ok, as long as you agree about possible further changes to it :)
Sure.
Thanks
> Also, Mark went silent on the topic, so we don't know if he likes it.
>
> Rémy
Re: Introduce methods read/write with ByteBuffer in CoyoteInputStream/CoyoteOutputStream
Posted by Rémy Maucherat <re...@apache.org>.
2016-10-27 13:24 GMT+02:00 Violeta Georgieva <mi...@gmail.com>:
> So are you OK to have in Tomcat 8.5 the read/write with ByteBuffer in
> CoyoteInputStream/CoyoteOutputStream?
> The main purpose for this refactoring was to introduce them without
> regression in the performance.
> I would like to have these APIs in Tomcat 8.5.
>
> Ok, as long as you agree about possible further changes to it :) Also,
Mark went silent on the topic, so we don't know if he likes it.
Rémy
Re: Introduce methods read/write with ByteBuffer in CoyoteInputStream/CoyoteOutputStream
Posted by Violeta Georgieva <mi...@gmail.com>.
2016-10-27 0:44 GMT+03:00 Rémy Maucherat <re...@apache.org>:
>
> 2016-10-26 20:45 GMT+02:00 Violeta Georgieva <mi...@gmail.com>:
>
> > Hi Remy,
> >
> > 2016-10-25 1:49 GMT+03:00 Rémy Maucherat <re...@apache.org>:
> > >
> > > 2016-10-24 20:18 GMT+02:00 Violeta Georgieva <mi...@gmail.com>:
> > >
> > > > There are no issues reported for the refactored functionality
available
> > in
> > > > 9.0.0.M11.
> > > > I think it is safe to back port the changes for the next 8.5
release.
> > > > Do you see any issues?
> > > >
> > > > Ok, so it probably works. I suppose it's up to me, at some point, to
> > try
> > > to improve whatever I would like to improve (for example, it lost API
> > > symmetry - normally, read/write APIs are the same, not the case here
-,
> > and
> > > it's not very GC friendly).
> >
> > Regarding API symmetry: for the read API I cannot use ByteBuffer
directly
> > as parameter so the option is to change the write API to use
> > ApplicationBufferHandler.
> > This change can be done easily but the drawback is that one additional
> > wrapper object will be created for each Response object.
> > If you are OK with this I can make it.
> >
>
> No. Since my own async scatter/gather didn't provide any benefits, unless
> something unexpected occurs I'll give up on that and could try to rework
> this at some point.
So are you OK to have in Tomcat 8.5 the read/write with ByteBuffer in
CoyoteInputStream/CoyoteOutputStream?
The main purpose for this refactoring was to introduce them without
regression in the performance.
I would like to have these APIs in Tomcat 8.5.
Thanks,
Violeta
> >
> > Let me know if you have other requirements for this change so that I can
> > implement them and after that backport the change in Tomcat 8.5.
> >
> > I will double check my tests again for GC issues. Do you have some
> > observations or concrete concerns so I can focus on them?
> >
> > It does duplicate ByteBuffers, that creates some objects. it doesn't
have
> any measurable impact on performance however since I didn't notice
anything.
>
> Rémy
Re: Introduce methods read/write with ByteBuffer in CoyoteInputStream/CoyoteOutputStream
Posted by Rémy Maucherat <re...@apache.org>.
2016-10-26 20:45 GMT+02:00 Violeta Georgieva <mi...@gmail.com>:
> Hi Remy,
>
> 2016-10-25 1:49 GMT+03:00 Rémy Maucherat <re...@apache.org>:
> >
> > 2016-10-24 20:18 GMT+02:00 Violeta Georgieva <mi...@gmail.com>:
> >
> > > There are no issues reported for the refactored functionality available
> in
> > > 9.0.0.M11.
> > > I think it is safe to back port the changes for the next 8.5 release.
> > > Do you see any issues?
> > >
> > > Ok, so it probably works. I suppose it's up to me, at some point, to
> try
> > to improve whatever I would like to improve (for example, it lost API
> > symmetry - normally, read/write APIs are the same, not the case here -,
> and
> > it's not very GC friendly).
>
> Regarding API symmetry: for the read API I cannot use ByteBuffer directly
> as parameter so the option is to change the write API to use
> ApplicationBufferHandler.
> This change can be done easily but the drawback is that one additional
> wrapper object will be created for each Response object.
> If you are OK with this I can make it.
>
No. Since my own async scatter/gather didn't provide any benefits, unless
something unexpected occurs I'll give up on that and could try to rework
this at some point.
>
> Let me know if you have other requirements for this change so that I can
> implement them and after that backport the change in Tomcat 8.5.
>
> I will double check my tests again for GC issues. Do you have some
> observations or concrete concerns so I can focus on them?
>
> It does duplicate ByteBuffers, that creates some objects. it doesn't have
any measurable impact on performance however since I didn't notice anything.
Rémy
Re: Introduce methods read/write with ByteBuffer in CoyoteInputStream/CoyoteOutputStream
Posted by Violeta Georgieva <mi...@gmail.com>.
Hi Remy,
2016-10-25 1:49 GMT+03:00 Rémy Maucherat <re...@apache.org>:
>
> 2016-10-24 20:18 GMT+02:00 Violeta Georgieva <mi...@gmail.com>:
>
> > There are no issues reported for the refactored functionality available
in
> > 9.0.0.M11.
> > I think it is safe to back port the changes for the next 8.5 release.
> > Do you see any issues?
> >
> > Ok, so it probably works. I suppose it's up to me, at some point, to try
> to improve whatever I would like to improve (for example, it lost API
> symmetry - normally, read/write APIs are the same, not the case here -,
and
> it's not very GC friendly).
Regarding API symmetry: for the read API I cannot use ByteBuffer directly
as parameter so the option is to change the write API to use
ApplicationBufferHandler.
This change can be done easily but the drawback is that one additional
wrapper object will be created for each Response object.
If you are OK with this I can make it.
Let me know if you have other requirements for this change so that I can
implement them and after that backport the change in Tomcat 8.5.
I will double check my tests again for GC issues. Do you have some
observations or concrete concerns so I can focus on them?
Thanks,
Violeta
> Rémy
Re: Introduce methods read/write with ByteBuffer in CoyoteInputStream/CoyoteOutputStream
Posted by Rémy Maucherat <re...@apache.org>.
2016-10-24 20:18 GMT+02:00 Violeta Georgieva <mi...@gmail.com>:
> There are no issues reported for the refactored functionality available in
> 9.0.0.M11.
> I think it is safe to back port the changes for the next 8.5 release.
> Do you see any issues?
>
> Ok, so it probably works. I suppose it's up to me, at some point, to try
to improve whatever I would like to improve (for example, it lost API
symmetry - normally, read/write APIs are the same, not the case here -, and
it's not very GC friendly).
Rémy
Re: Introduce methods read/write with ByteBuffer in CoyoteInputStream/CoyoteOutputStream
Posted by Violeta Georgieva <mi...@gmail.com>.
Hi Remy,
2016-10-09 22:34 GMT+03:00 Violeta Georgieva <mi...@gmail.com>:
>
> Hi,
>
> 2016-10-03 18:21 GMT+03:00 Rémy Maucherat <re...@apache.org>:
> >
> > 2016-10-03 16:58 GMT+02:00 Violeta Georgieva <mi...@gmail.com>:
> >
> > > Does this means that you are Ok to back port the ByteBuffer
replacement in
> > > Tomcat 8.5?
> > >
> > > I don't want to sound too annoying, but it's not tested enough and
8.5 is
> > stable. Also, it's not done yet, right ? I think it should be complete
> > (InputBuffer ?)
>
> I think it is completed now.
>
> > and go through one 9M release.
>
There are no issues reported for the refactored functionality available in
9.0.0.M11.
I think it is safe to back port the changes for the next 8.5 release.
Do you see any issues?
Thanks,
Violeta
> OK
>
> Regards,
> Violeta
>
> >
> > Rémy
Re: Introduce methods read/write with ByteBuffer in CoyoteInputStream/CoyoteOutputStream
Posted by Violeta Georgieva <mi...@gmail.com>.
Hi,
2016-10-03 18:21 GMT+03:00 Rémy Maucherat <re...@apache.org>:
>
> 2016-10-03 16:58 GMT+02:00 Violeta Georgieva <mi...@gmail.com>:
>
> > Does this means that you are Ok to back port the ByteBuffer replacement
in
> > Tomcat 8.5?
> >
> > I don't want to sound too annoying, but it's not tested enough and 8.5
is
> stable. Also, it's not done yet, right ? I think it should be complete
> (InputBuffer ?)
I think it is completed now.
> and go through one 9M release.
OK
Regards,
Violeta
>
> Rémy
Re: Introduce methods read/write with ByteBuffer in CoyoteInputStream/CoyoteOutputStream
Posted by Rémy Maucherat <re...@apache.org>.
2016-10-03 16:58 GMT+02:00 Violeta Georgieva <mi...@gmail.com>:
> Does this means that you are Ok to back port the ByteBuffer replacement in
> Tomcat 8.5?
>
> I don't want to sound too annoying, but it's not tested enough and 8.5 is
stable. Also, it's not done yet, right ? I think it should be complete
(InputBuffer ?) and go through one 9M release.
Rémy
Re: Introduce methods read/write with ByteBuffer in CoyoteInputStream/CoyoteOutputStream
Posted by Violeta Georgieva <mi...@gmail.com>.
2016-10-03 17:53 GMT+03:00 Rémy Maucherat <re...@apache.org>:
>
> 2016-10-03 15:57 GMT+02:00 Violeta Georgieva <mi...@gmail.com>:
>
> > Hi Remy,
> >
> > Did you manage to check or test the changed that I made?
> > In case you have doubts for some scenarios let me know and I will test
it.
> >
> > Hum, I tested plain text as well as OpenSSL, this should (hopefully)
cover
> the worst cases. Although in theory it could be slower, I cannot actually
> measure any difference. So that should be good enough to continue with
that
> ByteBuffer replacement.
Does this means that you are Ok to back port the ByteBuffer replacement in
Tomcat 8.5?
Thanks,
Violeta
> Rémy
Re: Introduce methods read/write with ByteBuffer in CoyoteInputStream/CoyoteOutputStream
Posted by Rémy Maucherat <re...@apache.org>.
2016-10-03 15:57 GMT+02:00 Violeta Georgieva <mi...@gmail.com>:
> Hi Remy,
>
> Did you manage to check or test the changed that I made?
> In case you have doubts for some scenarios let me know and I will test it.
>
> Hum, I tested plain text as well as OpenSSL, this should (hopefully) cover
the worst cases. Although in theory it could be slower, I cannot actually
measure any difference. So that should be good enough to continue with that
ByteBuffer replacement.
Rémy
Re: Introduce methods read/write with ByteBuffer in CoyoteInputStream/CoyoteOutputStream
Posted by Violeta Georgieva <mi...@gmail.com>.
Hi Remy,
2016-09-29 0:08 GMT+03:00 Violeta Georgieva <mi...@gmail.com>:
>
> Hi,
>
>
> 2016-09-22 22:21 GMT+03:00 Violeta Georgieva <mi...@gmail.com>:
> >
> >
> >
> > 2016-09-22 18:29 GMT+03:00 Rémy Maucherat <re...@apache.org>:
> > >
> > > 2016-09-22 17:13 GMT+02:00 Violeta Georgieva <mi...@gmail.com>:
> > >
> > > > Hi,
> > > >
> > > > 2016-09-14 20:43 GMT+03:00 Rémy Maucherat <re...@apache.org>:
> > > > >
> > > > > 2016-09-13 20:07 GMT+02:00 Violeta Georgieva <mi...@gmail.com>:
> > > > >
> > > > > > I would like to back port these changes to Tomcat 8.5 if you do
not
> > > > have
> > > > > > any concerns.
> > > > > >
> > > > > > -0.1. I think this is ok, but not certain yet.
> > > > > - Performance is ok for the "common" unencrypted scenario, but it
could
> > > > go
> > > > > down if using SSL, for example [= if the need for direct buffers
could be
> > > > > hurting - if it is, then there's a problem since SSL will be used
almost
> > > > > all the time moving forwards].
> > > >
> > > > I executed many combinations of performance scenarios - using direct
> > > > ByteBuffer/non direct ByteBuffer,
> > > > HTTP/HTTPS, sslImplementationName -
> > > > org.apache.tomcat.util.net.jsse.JSSEImplementation/org.
> > > > apache.tomcat.util.net.openssl.OpenSSLImplementation
> > > > And results with/without my changes are the same.
> > > > I'm testing with original Tomcat 9.0.0.M9 and Tomcat 9.0.0.M9 +
only my
> > > > changes.
> > > > I'm testing with blocking and non blocking.
> > > >
> > > > Just for testing purposes I changed
> > > > org.apache.catalina.connector.OutputBuffer
> > > > to use direct ByteBuffer and then executed SSL scenarios and again
the
> > > > results are the same.
> > > > No visible performance improvement/degradation.
> > > >
> > > > Do you have some scenarios in mind in which a performance
degradation can
> > > > be experienced.
> > > > I can test them and take a deeper look.
> > > >
> > >
> > > Ok, good, normally it's the best thing to test. If using OpenSSL, you
> > > should see better performance with direct buffers though, so maybe
there's
> > > something "wrong" with the testing. If you plan more ByteBuffer use,
then
> > > go ahead and I'll test it.
> >
> > My idea was to introduce the changes step by step,
> > because of this I wanted to backport the current changes to Tomcat 8.5
and receive a feedback.
> > But if you propose to introduce all ByteBuffer related changes
together, I'm OK and can commit the next changes.
> > I saw several places where switch to ByteBuffer can be done and I'm
going to finish and commit it.
>
>
> I committed the changes that I planed. I executed several combinations of
tests (performance and regression).
> There are neither issues nor performance degradation.
>
> Do you see some risks with this change and performance scenario that
should be executed?
Did you manage to check or test the changed that I made?
In case you have doubts for some scenarios let me know and I will test it.
Regards,
Violeta
> Thanks,
> Violeta
>
>
> > > >
> > > >
> > > > > - I didn't test again after the header processing refactoring
(the old
> > > > code
> > > > > was there for speed back then in early 2000s, most likely it
doesn't make
> > > > > any difference now but it could be verified as well).
> > > >
> > > > I executed many tests and didn't see any problems.
> > > >
> > >
> > > Yes, I had verified this one already :)
> >
> > I really appreciate your reviews and testing.
> >
> > Thanks,
> > Violeta
> >
> >
> > > Rémy
Re: Introduce methods read/write with ByteBuffer in CoyoteInputStream/CoyoteOutputStream
Posted by Violeta Georgieva <mi...@gmail.com>.
Hi,
2016-09-22 22:21 GMT+03:00 Violeta Georgieva <mi...@gmail.com>:
>
>
>
> 2016-09-22 18:29 GMT+03:00 Rémy Maucherat <re...@apache.org>:
> >
> > 2016-09-22 17:13 GMT+02:00 Violeta Georgieva <mi...@gmail.com>:
> >
> > > Hi,
> > >
> > > 2016-09-14 20:43 GMT+03:00 Rémy Maucherat <re...@apache.org>:
> > > >
> > > > 2016-09-13 20:07 GMT+02:00 Violeta Georgieva <mi...@gmail.com>:
> > > >
> > > > > I would like to back port these changes to Tomcat 8.5 if you do
not
> > > have
> > > > > any concerns.
> > > > >
> > > > > -0.1. I think this is ok, but not certain yet.
> > > > - Performance is ok for the "common" unencrypted scenario, but it
could
> > > go
> > > > down if using SSL, for example [= if the need for direct buffers
could be
> > > > hurting - if it is, then there's a problem since SSL will be used
almost
> > > > all the time moving forwards].
> > >
> > > I executed many combinations of performance scenarios - using direct
> > > ByteBuffer/non direct ByteBuffer,
> > > HTTP/HTTPS, sslImplementationName -
> > > org.apache.tomcat.util.net.jsse.JSSEImplementation/org.
> > > apache.tomcat.util.net.openssl.OpenSSLImplementation
> > > And results with/without my changes are the same.
> > > I'm testing with original Tomcat 9.0.0.M9 and Tomcat 9.0.0.M9 + only
my
> > > changes.
> > > I'm testing with blocking and non blocking.
> > >
> > > Just for testing purposes I changed
> > > org.apache.catalina.connector.OutputBuffer
> > > to use direct ByteBuffer and then executed SSL scenarios and again the
> > > results are the same.
> > > No visible performance improvement/degradation.
> > >
> > > Do you have some scenarios in mind in which a performance degradation
can
> > > be experienced.
> > > I can test them and take a deeper look.
> > >
> >
> > Ok, good, normally it's the best thing to test. If using OpenSSL, you
> > should see better performance with direct buffers though, so maybe
there's
> > something "wrong" with the testing. If you plan more ByteBuffer use,
then
> > go ahead and I'll test it.
>
> My idea was to introduce the changes step by step,
> because of this I wanted to backport the current changes to Tomcat 8.5
and receive a feedback.
> But if you propose to introduce all ByteBuffer related changes together,
I'm OK and can commit the next changes.
> I saw several places where switch to ByteBuffer can be done and I'm going
to finish and commit it.
I committed the changes that I planed. I executed several combinations of
tests (performance and regression).
There are neither issues nor performance degradation.
Do you see some risks with this change and performance scenario that should
be executed?
Thanks,
Violeta
> > >
> > >
> > > > - I didn't test again after the header processing refactoring (the
old
> > > code
> > > > was there for speed back then in early 2000s, most likely it
doesn't make
> > > > any difference now but it could be verified as well).
> > >
> > > I executed many tests and didn't see any problems.
> > >
> >
> > Yes, I had verified this one already :)
>
> I really appreciate your reviews and testing.
>
> Thanks,
> Violeta
>
>
> > Rémy
Re: Introduce methods read/write with ByteBuffer in CoyoteInputStream/CoyoteOutputStream
Posted by Violeta Georgieva <mi...@gmail.com>.
2016-09-22 18:29 GMT+03:00 Rémy Maucherat <re...@apache.org>:
>
> 2016-09-22 17:13 GMT+02:00 Violeta Georgieva <mi...@gmail.com>:
>
> > Hi,
> >
> > 2016-09-14 20:43 GMT+03:00 Rémy Maucherat <re...@apache.org>:
> > >
> > > 2016-09-13 20:07 GMT+02:00 Violeta Georgieva <mi...@gmail.com>:
> > >
> > > > I would like to back port these changes to Tomcat 8.5 if you do not
> > have
> > > > any concerns.
> > > >
> > > > -0.1. I think this is ok, but not certain yet.
> > > - Performance is ok for the "common" unencrypted scenario, but it
could
> > go
> > > down if using SSL, for example [= if the need for direct buffers
could be
> > > hurting - if it is, then there's a problem since SSL will be used
almost
> > > all the time moving forwards].
> >
> > I executed many combinations of performance scenarios - using direct
> > ByteBuffer/non direct ByteBuffer,
> > HTTP/HTTPS, sslImplementationName -
> > org.apache.tomcat.util.net.jsse.JSSEImplementation/org.
> > apache.tomcat.util.net.openssl.OpenSSLImplementation
> > And results with/without my changes are the same.
> > I'm testing with original Tomcat 9.0.0.M9 and Tomcat 9.0.0.M9 + only my
> > changes.
> > I'm testing with blocking and non blocking.
> >
> > Just for testing purposes I changed
> > org.apache.catalina.connector.OutputBuffer
> > to use direct ByteBuffer and then executed SSL scenarios and again the
> > results are the same.
> > No visible performance improvement/degradation.
> >
> > Do you have some scenarios in mind in which a performance degradation
can
> > be experienced.
> > I can test them and take a deeper look.
> >
>
> Ok, good, normally it's the best thing to test. If using OpenSSL, you
> should see better performance with direct buffers though, so maybe there's
> something "wrong" with the testing. If you plan more ByteBuffer use, then
> go ahead and I'll test it.
My idea was to introduce the changes step by step,
because of this I wanted to backport the current changes to Tomcat 8.5 and
receive a feedback.
But if you propose to introduce all ByteBuffer related changes together,
I'm OK and can commit the next changes.
I saw several places where switch to ByteBuffer can be done and I'm going
to finish and commit it.
> >
> >
> > > - I didn't test again after the header processing refactoring (the old
> > code
> > > was there for speed back then in early 2000s, most likely it doesn't
make
> > > any difference now but it could be verified as well).
> >
> > I executed many tests and didn't see any problems.
> >
>
> Yes, I had verified this one already :)
I really appreciate your reviews and testing.
Thanks,
Violeta
> Rémy
Re: Introduce methods read/write with ByteBuffer in CoyoteInputStream/CoyoteOutputStream
Posted by Rémy Maucherat <re...@apache.org>.
2016-09-22 17:13 GMT+02:00 Violeta Georgieva <mi...@gmail.com>:
> Hi,
>
> 2016-09-14 20:43 GMT+03:00 Rémy Maucherat <re...@apache.org>:
> >
> > 2016-09-13 20:07 GMT+02:00 Violeta Georgieva <mi...@gmail.com>:
> >
> > > I would like to back port these changes to Tomcat 8.5 if you do not
> have
> > > any concerns.
> > >
> > > -0.1. I think this is ok, but not certain yet.
> > - Performance is ok for the "common" unencrypted scenario, but it could
> go
> > down if using SSL, for example [= if the need for direct buffers could be
> > hurting - if it is, then there's a problem since SSL will be used almost
> > all the time moving forwards].
>
> I executed many combinations of performance scenarios - using direct
> ByteBuffer/non direct ByteBuffer,
> HTTP/HTTPS, sslImplementationName -
> org.apache.tomcat.util.net.jsse.JSSEImplementation/org.
> apache.tomcat.util.net.openssl.OpenSSLImplementation
> And results with/without my changes are the same.
> I'm testing with original Tomcat 9.0.0.M9 and Tomcat 9.0.0.M9 + only my
> changes.
> I'm testing with blocking and non blocking.
>
> Just for testing purposes I changed
> org.apache.catalina.connector.OutputBuffer
> to use direct ByteBuffer and then executed SSL scenarios and again the
> results are the same.
> No visible performance improvement/degradation.
>
> Do you have some scenarios in mind in which a performance degradation can
> be experienced.
> I can test them and take a deeper look.
>
Ok, good, normally it's the best thing to test. If using OpenSSL, you
should see better performance with direct buffers though, so maybe there's
something "wrong" with the testing. If you plan more ByteBuffer use, then
go ahead and I'll test it.
>
>
> > - I didn't test again after the header processing refactoring (the old
> code
> > was there for speed back then in early 2000s, most likely it doesn't make
> > any difference now but it could be verified as well).
>
> I executed many tests and didn't see any problems.
>
Yes, I had verified this one already :)
Rémy
Re: Introduce methods read/write with ByteBuffer in CoyoteInputStream/CoyoteOutputStream
Posted by Violeta Georgieva <mi...@gmail.com>.
Hi,
2016-09-14 20:43 GMT+03:00 Rémy Maucherat <re...@apache.org>:
>
> 2016-09-13 20:07 GMT+02:00 Violeta Georgieva <mi...@gmail.com>:
>
> > I would like to back port these changes to Tomcat 8.5 if you do not have
> > any concerns.
> >
> > -0.1. I think this is ok, but not certain yet.
> - Performance is ok for the "common" unencrypted scenario, but it could go
> down if using SSL, for example [= if the need for direct buffers could be
> hurting - if it is, then there's a problem since SSL will be used almost
> all the time moving forwards].
I executed many combinations of performance scenarios - using direct
ByteBuffer/non direct ByteBuffer,
HTTP/HTTPS, sslImplementationName -
org.apache.tomcat.util.net.jsse.JSSEImplementation/org.apache.tomcat.util.net.openssl.OpenSSLImplementation
And results with/without my changes are the same.
I'm testing with original Tomcat 9.0.0.M9 and Tomcat 9.0.0.M9 + only my
changes.
I'm testing with blocking and non blocking.
Just for testing purposes I changed
org.apache.catalina.connector.OutputBuffer
to use direct ByteBuffer and then executed SSL scenarios and again the
results are the same.
No visible performance improvement/degradation.
Do you have some scenarios in mind in which a performance degradation can
be experienced.
I can test them and take a deeper look.
> - I didn't test again after the header processing refactoring (the old
code
> was there for speed back then in early 2000s, most likely it doesn't make
> any difference now but it could be verified as well).
I executed many tests and didn't see any problems.
Regards,
Violeta
> Rémy
Re: Introduce methods read/write with ByteBuffer in CoyoteInputStream/CoyoteOutputStream
Posted by Rémy Maucherat <re...@apache.org>.
2016-09-13 20:07 GMT+02:00 Violeta Georgieva <mi...@gmail.com>:
> I would like to back port these changes to Tomcat 8.5 if you do not have
> any concerns.
>
> -0.1. I think this is ok, but not certain yet.
- Performance is ok for the "common" unencrypted scenario, but it could go
down if using SSL, for example [= if the need for direct buffers could be
hurting - if it is, then there's a problem since SSL will be used almost
all the time moving forwards].
- I didn't test again after the header processing refactoring (the old code
was there for speed back then in early 2000s, most likely it doesn't make
any difference now but it could be verified as well).
Rémy
Re: Introduce methods read/write with ByteBuffer in CoyoteInputStream/CoyoteOutputStream
Posted by Violeta Georgieva <mi...@gmail.com>.
Hi,
2016-08-30 15:35 GMT+03:00 Violeta Georgieva <mi...@gmail.com>:
>
> Hi,
>
> 2016-08-24 16:44 GMT+03:00 Violeta Georgieva <vi...@apache.org>:
> >
> > Hi,
> >
> > Currently it is not possible to use
ServletInputStream.read/ServletOutputStream.write methods with ByteBuffer.
To enable such scenarios in Tomcat I would like to extend the Tomcat’s
internal implementation (CoyoteInputStream/CoyoteOutputStream). I already
did some refactoring in ByteChunk/CharChunk/SocketWrapperBase so that I can
introduce read/write with ByteBuffer. I executed Autobahn test suite to
check the performance impact after the refactoring and it showed no
performance regression. If there are no objections I want to commit these
changes in Tomcat 9 and Tomcat 8.5.x.
> >
>
> I introduced CoyoteOutputStream.write(ByteBuffer) it uses new methods
with ByteBuffer instead of ByteChunk.
> Next step is to replace ByteChunk/CharChunk usage in CoyoteOutputStream
with ByteBuffer/CharBuffer thus I will switch to the new methods and all
CoyoteOutputStream.write method will use them.
>
I would like to back port these changes to Tomcat 8.5 if you do not have
any concerns.
Thanks,
Violeta
> Regards,
> Violeta
>
> > Best Regards,
> > Violeta
Re: Introduce methods read/write with ByteBuffer in CoyoteInputStream/CoyoteOutputStream
Posted by Violeta Georgieva <mi...@gmail.com>.
Hi,
2016-08-24 16:44 GMT+03:00 Violeta Georgieva <vi...@apache.org>:
>
> Hi,
>
> Currently it is not possible to use
ServletInputStream.read/ServletOutputStream.write methods with ByteBuffer.
To enable such scenarios in Tomcat I would like to extend the Tomcat’s
internal implementation (CoyoteInputStream/CoyoteOutputStream). I already
did some refactoring in ByteChunk/CharChunk/SocketWrapperBase so that I can
introduce read/write with ByteBuffer. I executed Autobahn test suite to
check the performance impact after the refactoring and it showed no
performance regression. If there are no objections I want to commit these
changes in Tomcat 9 and Tomcat 8.5.x.
>
I introduced CoyoteOutputStream.write(ByteBuffer) it uses new methods with
ByteBuffer instead of ByteChunk.
Next step is to replace ByteChunk/CharChunk usage in CoyoteOutputStream
with ByteBuffer/CharBuffer thus I will switch to the new methods and all
CoyoteOutputStream.write method will use them.
Regards,
Violeta
> Best Regards,
> Violeta