@taleinat If you want you want to use this feel free to rebase against my branch. This PR is mainly because as part of figuring out how to make your PR fast, I actually re-wrote your PR, so it seemed easier to just push my changes.

I've merged in the tests and NEWS from #8859, but I now think this PR should be merged instead of that one.

Comparing performance (using the script from this comment) of this PR (updated after sanitize_isoformat_str refactor):

datetime constructor:                1192.5ns
fromisoformat:                       561.3ns
fromisoformat (special characters):  599.7ns
fromisoformat (non-utf8):            1289.5ns
fromisoformat (fail, non-utf8):      3501.5ns
fromisoformat (fail, utf8):          1738.7ns

Compared with #8859:

datetime constructor:                1165.1ns
fromisoformat:                       520.7ns
fromisoformat (special characters):  1153.1ns
fromisoformat (non-utf8):            1165.8ns
fromisoformat (fail, non-utf8):      2815.5ns
fromisoformat (fail, utf8):          1648.3ns

It's much faster in at least one common(ish) case (utf-8) and essentially the same performance in all other cases. IMO, this one also is more readable, since it's essentially equivalent to:

def new_isoformat(dtstr):
    if len(dtstr) > 10 and is_surrogate(dtstr[10]):
        dtstr = "%sT%s" % (dtstr[0:10], dtstr[11:])
    return old_isoformat_minus_segfaults(dtstr)

It does not require the more complicated fast-path/slow-path branching in #8859 and proliferation of intermediate PyObjects (and associated refcounts) is kept to an absolute minimum.

CC @abalkin @serhiy-storchaka

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

I believe all the changes are fixed. Thanks for the review @taleinat

@pganssle, for your consideration: If you also do the UTF-8 encoding in _sanitize_isoformat_str (renamed appropriately) and return a const char * and length, you can avoid the conditional Py_DECREF() in the fast-path at the end of datetime_fromisoformat().

@taleinat I'm not entirely sure, but I think that wouldn't work; when the RC of the temporary dtstr reaches 0 it would be deleted, and I think that the temporary dtstr is managing the memory for dt_ptr. If that's not how it works, I'm not sure what would be managing that memory, since I'm not allocating any memory for it, or freeing it later.

@pganssle, you're right, I hadn't considered that. Better to leave it as it is then.

Just remove the PyUnicode_GetLength() call and it should be ready to go in.

Also, you're welcome to add yourself to the NEWS section, i.e. "Patch by Paul Ganssle".

@taleinat Fixed the duplicate PyUnicode_GetLength and the missing NULL check. I don't really need to be mentioned in the NEWS, plus I think it would be complicated to properly assign credit for this patch, as it was a collaborative effort between me, you and @izbyshev.

@pganssle, looks good!

I'm a core dev and will merge this so my name will be on it anyways.

Thanks @pganssle for the PR, and @taleinat for merging it 🌮🎉.. I'm working now to backport this PR to: 3.7.
🐍🍒⛏🤖

GH-8877 is a backport of this pull request to the 3.7 branch.

@serhiy-storchaka I'll make a second PR with the cleanup.