Repository navigation
perf: build the StatusError message on first use - #180
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
StatusError.__init__used to format"<status> <method> <redacted url>"as soon as the error was built. That meantstr(request.url)plusredact_urlon every 4xx/5xx, andRetrypaid it again on every retried attempt, even though nothing reads the message unless the error is printed or logged.Now
__init__only storesresponse.BaseException.__new__has already put(response,)in the C-levelargsslot.StatusError.argsis a property: the first time it is read and still holds that(response,), it builds the summary, writes(summary,)back into the slot, and returns it. Later reads return the cached tuple.__str__readsself.args, sostr(), tracebacks andloggingall go through the same path. Assigningexc.argsstill works, and the assigned value replaces the message the way it does on any exception.Nothing a user can see changes:
exc.argsis still("<status> <method> <redacted url>",). A cheaper-looking option was to skipsuper().__init__and leaveargsas(response,). Its raise cost is the same, because the property only runs when someone readsargs, but it changesargsin a way users can see, so I did not take it.str(exc),repr(exc),traceback.format_exception, the uncaught-exception hook andloggingexc_infooutput are byte-identical tomainforStatusError,ClientStatusError,ServerStatusErrorand every class inSTATUS_TO_EXCEPTION. I checked this with URLs that have userinfo, a port, an IPv6 host, query values and fragment parameters, with and withoutadd_note()and a user attribute.copy.copyandcopy.deepcopyround-trip unchanged.ClientError.__reduce__readsself.args, which builds the message, so the restored instance carries the same redacted string, and__dict__holds no new keys._summary()as before.Measured on 3.14, best of 9, three alternating runs (the machine is noisy):
str(), plain URLstr(), userinfo and queryWhen the message is read, the property costs about 0.2 to 0.3 us extra. That path only runs when an error is printed or logged.
Dropped: resolver memoization
I also measured caching the resolved decoder per model. It saved about 0.2 to 1.3 us per
resolve()and under 1% end to end, and I dropped it because it would call a custom decoder'scan_decodeonce per model instead of once per request.Behaviour change for the 0.22.0 notes
None.
Tests
test_errors.py:argsholds the redacted summary, construction does not callredact_url, and assignedargsof length 0, 1 and 2 replace the message.