classification
Title: Not very good strcpy implementation in cpython/Python/strdup.c
Type: performance Stage: resolved
Components: C API Versions: Python 3.10
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: benjamin.peterson, fj92f3jj923f923, methane, shihai1991
Priority: normal Keywords: patch

Created on 2020-07-19 10:36 by fj92f3jj923f923, last changed 2020-07-27 03:29 by methane. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 21544 closed fj92f3jj923f923, 2020-07-19 10:36
PR 21634 merged fj92f3jj923f923, 2020-07-27 01:46
Messages (11)
msg373955 - (view) Author: (fj92f3jj923f923) * Date: 2020-07-19 10:36
Hi, all!

strdup implementation inside cpython/Python/strdup.c is not the best one.
It calls strlen + strcpy, which is the same as calling strlen twice + memcpy.
So I replaced it by the call of strlen + memcpy.

It is easy to look any implementation in any library.
Here for example:
https://code.woboq.org/userspace/glibc/string/strdup.c.html

So I fixed it here:
https://github.com/python/cpython/pull/21544
msg373976 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2020-07-19 23:54
I don't think we need to spend much effort on this implementation, since it will only be used if the system libc doesn't have a strdup already.
msg373986 - (view) Author: (fj92f3jj923f923) * Date: 2020-07-20 06:23
Got it.
But the fix is quite easy as I see, and does not require much affort too.
msg373989 - (view) Author: Inada Naoki (methane) * (Python committer) Date: 2020-07-20 06:58
Can we just remove strdup.c? How about hypot.c?
msg374199 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2020-07-24 18:26
Either remove it or close this issue as won’t fix.
msg374241 - (view) Author: Hai Shi (shihai1991) * (Python triager) Date: 2020-07-25 05:00
What's change for the performance benchmark?
msg374302 - (view) Author: Inada Naoki (methane) * (Python committer) Date: 2020-07-26 05:34
@fj92f3jj923f923 Would you update the PR to remove the strdup.c?
msg374319 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2020-07-26 15:11
There’s probably also something in configure.in that can switch this on.
msg374345 - (view) Author: (fj92f3jj923f923) * Date: 2020-07-26 21:23
I can give a try with removing
Please, wait
Thanks everyone for giving a chance to make a PR :)
msg374353 - (view) Author: (fj92f3jj923f923) * Date: 2020-07-27 02:00
Created a new PR for removal of strdup (Hope it is correct)
https://github.com/python/cpython/pull/21634
Closed old one
msg374359 - (view) Author: Inada Naoki (methane) * (Python committer) Date: 2020-07-27 03:28
New changeset 5798f787779006a94a55ec74a86da4627de90146 by wasiher in branch 'master':
bpo-41340: Removed fallback implementation for strdup (GH-21634)
https://github.com/python/cpython/commit/5798f787779006a94a55ec74a86da4627de90146
History
Date User Action Args
2020-07-27 03:29:02methanesetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2020-07-27 03:28:52methanesetmessages: + msg374359
2020-07-27 02:00:03fj92f3jj923f923setmessages: + msg374353
2020-07-27 01:46:10fj92f3jj923f923setpull_requests: + pull_request20776
2020-07-26 21:23:49fj92f3jj923f923setmessages: + msg374345
2020-07-26 15:22:00gvanrossumsetnosy: - gvanrossum
2020-07-26 15:11:17gvanrossumsetmessages: + msg374319
2020-07-26 05:34:22methanesetmessages: + msg374302
2020-07-25 05:00:43shihai1991setnosy: + shihai1991
messages: + msg374241
2020-07-24 18:26:41gvanrossumsetnosy: + gvanrossum
messages: + msg374199
2020-07-20 06:58:57methanesetnosy: + methane
messages: + msg373989
2020-07-20 06:23:00fj92f3jj923f923setmessages: + msg373986
2020-07-19 23:54:49benjamin.petersonsetnosy: + benjamin.peterson
messages: + msg373976
2020-07-19 10:36:53fj92f3jj923f923setkeywords: + patch
stage: patch review
pull_requests: + pull_request20686
2020-07-19 10:36:04fj92f3jj923f923create