You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2021/03/31 10:44:58 UTC
[GitHub] [incubator-nuttx] anchao opened a new pull request #3259: libc/string/strstr(): Use Byte-Shift algorithm for very long needles
anchao opened a new pull request #3259:
URL: https://github.com/apache/incubator-nuttx/pull/3259
## Summary
libc/string: Use Byte-Shift algorithm for very long needles
Reference here:
https://github.com/taleinat/byteshift_strstr
Signed-off-by: chao.an <an...@xiaomi.com>
## Impact
## Testing
strstr(3) performance degrade in the case of long needles, (e.g curl/mbedtls parse the CA certificates)
CA certificates:
https://curl.se/docs/caextract.html
curl mbedtls_x509_crt_parse_file:
https://github.com/curl/curl/blob/3266b35bbe21c68dea0dc7ccd991eb028e6d360c/lib/vtls/mbedtls.c#L304
strstr:
https://github.com/ARMmbed/mbedtls/blob/e93095fe6bdc99818f71197e808d20244cd1e41a/library/x509_crt.c#L1461-L1464
https://github.com/ARMmbed/mbedtls/blob/e93095fe6bdc99818f71197e808d20244cd1e41a/library/pem.c#L246-L251
In the default implementation, curl's CA certificate parse will last about 4 - 4.5 *Seconds* in multiple strstr(3) processing, after optimization, reduced to 80 milliseconds.
--
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] [incubator-nuttx] v01d commented on pull request #3259: libc/string/strstr(): Use Byte-Shift algorithm for very long needles
Posted by GitBox <gi...@apache.org>.
v01d commented on pull request #3259:
URL: https://github.com/apache/incubator-nuttx/pull/3259#issuecomment-811076128
I agree. Having the author sign SGA/ICLA is the ideal case and we should always try to do that to minimize the amount of non-apache licensed third-party work. If that doesn't happen for some reason, of course the only right approach is following the original license terms (include the header and add the notice).
--
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] [incubator-nuttx] hartmannathan commented on pull request #3259: libc/string/strstr(): Use Byte-Shift algorithm for very long needles
Posted by GitBox <gi...@apache.org>.
hartmannathan commented on pull request #3259:
URL: https://github.com/apache/incubator-nuttx/pull/3259#issuecomment-811964952
We could do what we are already doing for memcpy() -- there is the option CONFIG_MEMCPY_VIK to optionally use the optimized memcpy() function by Daniel Vik, subject to accepting the licensing in the top-level LICENSE file. However, I agree that we should communicate with the author, tell them we like their work, and ask very nicely for permission to include it in NuttX, with the following two options:
1. Keep the original MIT license and offer this optimized strstr() as an option like CONFIG_MEMCPY_VIK.
2. Or, if the author wants to, get a ICLA or SGA and make this the default implementation.
--
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] [incubator-nuttx] anchao commented on pull request #3259: libc/string/strstr(): Use Byte-Shift algorithm for very long needles
Posted by GitBox <gi...@apache.org>.
anchao commented on pull request #3259:
URL: https://github.com/apache/incubator-nuttx/pull/3259#issuecomment-811082304
OK, I will try to get the ICLA sign from author
--
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] [incubator-nuttx] patacongo commented on pull request #3259: libc/string/strstr(): Use Byte-Shift algorithm for very long needles
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #3259:
URL: https://github.com/apache/incubator-nuttx/pull/3259#issuecomment-811051978
This algorithm is licensed under the open-source MIT license:
The MIT License (MIT)
Copyright (c) 2014-2015 Tal Einat
Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal in the Software without restriction, including without limitation the rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons to whom the Software is furnished to do so, subject to the following conditions:
The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software.
THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
--
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] [incubator-nuttx] v01d commented on pull request #3259: libc/string/strstr(): Use Byte-Shift algorithm for very long needles
Posted by GitBox <gi...@apache.org>.
v01d commented on pull request #3259:
URL: https://github.com/apache/incubator-nuttx/pull/3259#issuecomment-811070145
Making into draft until the licensing issue is resolved
--
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] [incubator-nuttx] hartmannathan commented on pull request #3259: libc/string/strstr(): Use Byte-Shift algorithm for very long needles
Posted by GitBox <gi...@apache.org>.
hartmannathan commented on pull request #3259:
URL: https://github.com/apache/incubator-nuttx/pull/3259#issuecomment-812087024
> > We could do what we are already doing for memcpy() -- there is the option CONFIG_MEMCPY_VIK to optionally use the optimized memcpy() function by Daniel Vik, subject to accepting the licensing in the top-level LICENSE file.
>
> This was added to NuttX many years ago... long before NuttX was an Apache project and subject to the rules and etiquette of the Apache Software Foundation. Hence, that is not a good example to follow. It is not the Apache way.
Apologies, I wasn't clear: I didn't mean just go ahead and do it; I meant _if_ we get the author's permission to include it in NuttX with the author's original MIT license, we should have a Kconfig option to choose which strstr() implementation to use, so people can decide whether they want to accept that license or not. That would be similar to the way VIK memcpy() is handled now, and it would be documented with the function, the Kconfig, and the LICENSE file. But of course we would only do it with the author's blessing.
--
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] [incubator-nuttx] patacongo commented on pull request #3259: libc/string/strstr(): Use Byte-Shift algorithm for very long needles
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #3259:
URL: https://github.com/apache/incubator-nuttx/pull/3259#issuecomment-811073434
First of all, I think we need to determine if this is a case if the inappropriate inclusion of 3rd part code without proper attribution. My understanding is that the copyright does not cover re-use of the algorithm, only the code. Seeing local variables with the same name is a clue that the code was re-used.
Courtesy and good engineering etiquette would, I think, demand that we acknowledge the author/inventor in some way and reflect this in the licensing. I think that to incorporate 3rd part party code we need to:
- Get some approval/concurrence from the author,
- Retain the author's license header as required by the terms of the license for code reuse
- Add the reference to the 3rd party code to the LICENSE file.
Is that right? We can include 3rd party code, we just need to do it correctly and ethically.
--
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] [incubator-nuttx] patacongo commented on pull request #3259: libc/string/strstr(): Use Byte-Shift algorithm for very long needles
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #3259:
URL: https://github.com/apache/incubator-nuttx/pull/3259#issuecomment-812012185
> We could do what we are already doing for memcpy() -- there is the option CONFIG_MEMCPY_VIK to optionally use the optimized memcpy() function by Daniel Vik, subject to accepting the licensing in the top-level LICENSE file.
This was added to NuttX many years ago... long before NuttX was an Apache project and subject to the rules and etiquette of the Apache Software Foundation. Hence, that is not a good example to follow. It is not the Apache way.
--
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] [incubator-nuttx] Ouss4 commented on pull request #3259: libc/string/strstr(): Use Byte-Shift algorithm for very long needles
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on pull request #3259:
URL: https://github.com/apache/incubator-nuttx/pull/3259#issuecomment-811087817
>Get some approval/concurrence from the author,
Retain the author's license header as required by the terms of the license for code reuse
Add the reference to the 3rd party code to the LICENSE file.
Why should we get approval from the author? Isn't the permission to use/copy/modify already in the license he distributed the code under?
My understanding is that we only need to do step 2 & 3.
--
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] [incubator-nuttx] patacongo commented on pull request #3259: libc/string/strstr(): Use Byte-Shift algorithm for very long needles
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #3259:
URL: https://github.com/apache/incubator-nuttx/pull/3259#issuecomment-811106927
> Yes, can just do 2 & 3, but we would have to use the original license. Asking author for ICLA/SGA means we can move on to apache, but it is not critical (assuming third-party license is compatible).
#1 ("Get some approval/concurrence from the author") is something that Justin McClean said we needed to do when we first started this project. I was always confused about that. Justin said that is what they did on the MyNewt project. I agree that it is not any kind of legal requirement but we were told that it the higher level of ethics the Apache projects were supposed to conform to.
This is in the dev list archives somewhere, but I cannot find the discussion thread.
Being an open, honest, ethical project is a good thing. Our object is to benefit all people not to screw over other open source contributors.
--
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] [incubator-nuttx] patacongo commented on pull request #3259: libc/string/strstr(): Use Byte-Shift algorithm for very long needles
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #3259:
URL: https://github.com/apache/incubator-nuttx/pull/3259#issuecomment-811065941
>
>
> @anchao you can try to contact the author and ask if he accepts to release it under Apache License.
It would still be 3rd party code unless he also signs an ICLA
--
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] [incubator-nuttx] patacongo commented on pull request #3259: libc/string/strstr(): Use Byte-Shift algorithm for very long needles
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #3259:
URL: https://github.com/apache/incubator-nuttx/pull/3259#issuecomment-811109303
> Yes, can just do 2 & 3, but we would have to use the original license. Asking author for ICLA/SGA means we can move on to apache, but it is not critical (assuming third-party license is compatible).
#1 is not to get an ICLA or SGA but just to make sure that the author is comfortable with the inclusion of his/her code. I don't think it is strictly a requirement either. Justin was pretty adamant that we do this at about last year at this time but I don't personally care.
--
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] [incubator-nuttx] patacongo commented on pull request #3259: libc/string/strstr(): Use Byte-Shift algorithm for very long needles
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #3259:
URL: https://github.com/apache/incubator-nuttx/pull/3259#issuecomment-811051601
This looks like third party code (since, for example, local variables have the same name). It is unethical to take other people's code without respecting their authorship and licensing and simply slap an Apache license on it and claim that you wrote it.
--
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] [incubator-nuttx] v01d commented on pull request #3259: libc/string/strstr(): Use Byte-Shift algorithm for very long needles
Posted by GitBox <gi...@apache.org>.
v01d commented on pull request #3259:
URL: https://github.com/apache/incubator-nuttx/pull/3259#issuecomment-811094268
> > Get some approval/concurrence from the author,
> > Retain the author's license header as required by the terms of the license for code reuse
> > Add the reference to the 3rd party code to the LICENSE file.
>
> Why should we get approval from the author? Isn't the permission to use/copy/modify already in the license he distributed the code under?
> My understanding is that we only need to do step 2 & 3.
Yes, can just do 2 & 3, but we would have to use the original license. Asking author for ICLA/SGA means we can move on to apache, but it is not critical (assuming third-party license is compatible).
--
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] [incubator-nuttx] acassis commented on pull request #3259: libc/string/strstr(): Use Byte-Shift algorithm for very long needles
Posted by GitBox <gi...@apache.org>.
acassis commented on pull request #3259:
URL: https://github.com/apache/incubator-nuttx/pull/3259#issuecomment-811064894
@anchao you can try to contact the author and ask if he accepts to release it under Apache License.
--
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