You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by GitBox <gi...@apache.org> on 2020/04/23 12:45:41 UTC
[GitHub] [tomcat] govi20 opened a new pull request #281: Change FilterChain holder from array to list
govi20 opened a new pull request #281:
URL: https://github.com/apache/tomcat/pull/281
Instead of resizing array manually, we can leverage `List` to hold filter-chain
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] govi20 commented on issue #281: Change FilterChain holder from array to list
Posted by GitBox <gi...@apache.org>.
govi20 commented on issue #281:
URL: https://github.com/apache/tomcat/pull/281#issuecomment-618439999
There are so many places in the codebase where we can replace array with List could reduce good amount of code.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] rmaucher commented on issue #281: Change FilterChain holder from array to list
Posted by GitBox <gi...@apache.org>.
rmaucher commented on issue #281:
URL: https://github.com/apache/tomcat/pull/281#issuecomment-618384315
This commit seems functionally useless but is solely motivated by personal preferences. At the very least, they should be accompanied by quick perf numbers that show equivalence or better.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] ChristopherSchultz commented on issue #281: Change FilterChain holder from array to list
Posted by GitBox <gi...@apache.org>.
ChristopherSchultz commented on issue #281:
URL: https://github.com/apache/tomcat/pull/281#issuecomment-618426909
There *is* some code-reduction, and therefore a reduction in the possibility of bugs. Obviously, this code is well-tested and does indeed work.
Another possible advantage is that the List can become unmodifiable at some point, which may possibly improve security and/or code-safety.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] rmaucher commented on issue #281: Change FilterChain holder from array to list
Posted by GitBox <gi...@apache.org>.
rmaucher commented on issue #281:
URL: https://github.com/apache/tomcat/pull/281#issuecomment-618465384
When I look at https://github.com/apache/tomcat/pull/271 in comparison, it has an extensive "why" demonstrating why it is actually better and it is not simply cosmetics.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] govi20 edited a comment on issue #281: Change FilterChain holder from array to list
Posted by GitBox <gi...@apache.org>.
govi20 edited a comment on issue #281:
URL: https://github.com/apache/tomcat/pull/281#issuecomment-618439999
There are so many places in the codebase where we can replace array with List and we could reduce good amount of code.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] markt-asf commented on issue #281: Change FilterChain holder from array to list
Posted by GitBox <gi...@apache.org>.
markt-asf commented on issue #281:
URL: https://github.com/apache/tomcat/pull/281#issuecomment-618447383
The question is "At what cost?" I have two concerns.
1. Regressions. The changes should be obvious and easy to check but there are plenty of examples in the history of Tomcat of seemingly simple clean-up triggering unintended regressions. That isn't a reason not to do it but there needs to be a clear benefit - such as reduced lines of code - of making the change.
2. Performance. I have in the past seen very significant performance drops changing from a simple array to an object (byte[] to ByteBuffer is the case I am thinking of). We need to be very careful of changes that are on the critical path (get executed for all/most requests). In those cases I would want to see repeatable performance numbers that demonstrated the performance cost/benefit of making the change including the impact on garbage generation. `ApplicationFilterChain` falls into this category.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org