perf: remove unnecessary c++ call with url.format #2045
Conversation
| return URLSerializer(this[kState].url) | ||
| return this[kState].url.toString() |
There was a problem hiding this comment.
@ronag Then we should at least keep the changes like this, where C++ call is not necessary at all.
There was a problem hiding this comment.
Why does URL.href go into C++ if it does the same thing as toString?
There was a problem hiding this comment.
toString() {
return this.#context.href;
}
get href() {
return this.#context.href;
}There is no difference?
There was a problem hiding this comment.
As far as I can see this is a noop.
There was a problem hiding this comment.
Ah I forgot that I changed it a couple of months ago (#1955). Regardless, this removes the extra branch (and is faster)
| @@ -117,7 +116,7 @@ class Response { | |||
| responseObject[kState].status = status | |||
|
|
|||
| // 6. Let value be parsedURL, serialized and isomorphic encoded. | |||
| const value = isomorphicEncode(URLSerializer(parsedURL)) | |||
| const value = isomorphicEncode(parsedURL.toString()) | |||
There was a problem hiding this comment.
@ronag Same place in here too.
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #2045 +/- ##
==========================================
- Coverage 90.39% 90.37% -0.02%
==========================================
Files 71 71
Lines 6184 6183 -1
==========================================
- Hits 5590 5588 -2
- Misses 594 595 +1
... and 1 file with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
Before
After