RESOLVED INVALID 181704
[Curl] Reuse same CurlRequest among authentication process.
https://bugs.webkit.org/show_bug.cgi?id=181704
Summary [Curl] Reuse same CurlRequest among authentication process.
Basuke Suzuki
Reported 2018-01-16 14:04:03 PST
When authentication is requested, currently a new CurlRequest is generated. For a sake of CPU and memory cost, it's better to reuse the same instance by implementing reset method.
Attachments
Patch (11.73 KB, patch)
2018-01-16 15:37 PST, Basuke Suzuki
no flags
Basuke Suzuki
Comment 1 2018-01-16 15:37:43 PST
Alex Christensen
Comment 2 2018-01-17 17:11:32 PST
Comment on attachment 331437 [details] Patch This kind of pattern adds a lot of places where if we add something new we will forget to update all the different ways of resetting it. Can you measure a significant performance improvement?
Basuke Suzuki
Comment 3 2018-01-18 16:59:35 PST
> This kind of pattern adds a lot of places where if we add something new we will forget to update all the different ways of resetting it. Can you measure a significant performance improvement? I agree with that concern, but beside those costs, we have an intention to reuse the request object for redirecting with following patch because some information are need to pass to the next communication. Without reset, we need to create a new instance and set those information. So that means we also have a risk if we forget to set those mandatery info . Thinking about the future safety, how about this: - we initialize values just before start communication even for the first communication. All initialization goes there. - those info we need to maintain among series of request are safe to be there.
Alex Christensen
Comment 4 2018-01-18 17:38:10 PST
What does this improve?
Basuke Suzuki
Comment 5 2018-01-19 11:47:42 PST
We are discussing about the cost we can save with this patch. As you suggested if the cost gain is small, we will close this bug. The result will be in next week.
Basuke Suzuki
Comment 6 2018-02-21 12:20:21 PST
This bug was obsoleted with a new plan. https://bugs.webkit.org/show_bug.cgi?id=183010
Note You need to log in before you can comment on or make changes to this bug.