You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by GitBox <gi...@apache.org> on 2020/06/03 11:28:25 UTC
[GitHub] [commons-compress] XenoAmess opened a new pull request #106: maybe we can add a break here
XenoAmess opened a new pull request #106:
URL: https://github.com/apache/commons-compress/pull/106
Hi.
Can we add a break here?
Also, should we try implement a data structure or something for doing this? a loop like this sounds slow.
If you don't mind I will give it a try several hours later (if I still have some time).
----------------------------------------------------------------
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
[GitHub] [commons-compress] bodewig merged pull request #106: [COMPRESS-535] maybe we can add a break here
Posted by GitBox <gi...@apache.org>.
bodewig merged pull request #106:
URL: https://github.com/apache/commons-compress/pull/106
----------------------------------------------------------------
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
[GitHub] [commons-compress] XenoAmess commented on pull request #106: [COMPRESS-535] maybe we can add a break here
Posted by GitBox <gi...@apache.org>.
XenoAmess commented on pull request #106:
URL: https://github.com/apache/commons-compress/pull/106#issuecomment-638181991
>In either case I would not go near changing the implementation without a decent performance test that proves the changed code is at least as fast as the old one used to be.
OK, I will find some time to do the test.
I will also try build a data structure for this question, and test performance for it too.
And, maybe we should not merge this pr until the test result comes?
----------------------------------------------------------------
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
[GitHub] [commons-compress] bodewig commented on pull request #106: [COMPRESS-535] maybe we can add a break here
Posted by GitBox <gi...@apache.org>.
bodewig commented on pull request #106:
URL: https://github.com/apache/commons-compress/pull/106#issuecomment-638632471
too late, I've already merged this one :-)
It is very much possible javac has become better in the fifteen years or so since the code has been written. A while ago I started https://github.com/bodewig/commons-compress-benchmarks as a limited JMH based setup to see how things changed over time. It might be a starting point - or not.
----------------------------------------------------------------
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
[GitHub] [commons-compress] bodewig commented on pull request #106: [COMPRESS-535] maybe we can add a break here
Posted by GitBox <gi...@apache.org>.
bodewig commented on pull request #106:
URL: https://github.com/apache/commons-compress/pull/106#issuecomment-638158417
@XenoAmess you don't need to create a JIRA ticket for every PR. I appreciate JIRA for bigger changes, but for a single break, I'm not sure. Oh, and please add a bit more context to the PR's titles :-)
The bzip2 code has been hand optimized with an unrolled loop years ago. It is quite possible this is no longer necessary and the JIT has become smart enough to do things "right". In either case I would not go near changing the implementation without a decent performance test that proves the changed code is at least as fast as the old one used to be. For many of our users speed is more important than elegance.
----------------------------------------------------------------
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
[GitHub] [commons-compress] coveralls commented on pull request #106: [COMPRESS-535] maybe we can add a break here
Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #106:
URL: https://github.com/apache/commons-compress/pull/106#issuecomment-638139331
[](https://coveralls.io/builds/31209625)
Coverage increased (+0.04%) to 87.325% when pulling **204faf58f69386d1b6f8d0410dcf2667d6d08051 on XenoAmess:should_we_add_break_here** into **f333bc647c860375cc4e9999da10b2c3c8d4d57f on apache:master**.
----------------------------------------------------------------
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
[GitHub] [commons-compress] bodewig commented on pull request #106: [COMPRESS-535] maybe we can add a break here
Posted by GitBox <gi...@apache.org>.
bodewig commented on pull request #106:
URL: https://github.com/apache/commons-compress/pull/106#issuecomment-638160654
oh, and thank you, of course
----------------------------------------------------------------
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