Bug 181704 - [Curl] Reuse same CurlRequest among authentication process.
Summary: [Curl] Reuse same CurlRequest among authentication process.
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Basuke Suzuki
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-01-16 14:04 PST by Basuke Suzuki
Modified: 2018-07-12 16:26 PDT (History)
5 users (show)

See Also:


Attachments
Patch (11.73 KB, patch)
2018-01-16 15:37 PST, Basuke Suzuki
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Basuke Suzuki 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.
Comment 1 Basuke Suzuki 2018-01-16 15:37:43 PST
Created attachment 331437 [details]
Patch
Comment 2 Alex Christensen 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?
Comment 3 Basuke Suzuki 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.
Comment 4 Alex Christensen 2018-01-18 17:38:10 PST
What does this improve?
Comment 5 Basuke Suzuki 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.
Comment 6 Basuke Suzuki 2018-02-21 12:20:21 PST
This bug was obsoleted with a new plan. https://bugs.webkit.org/show_bug.cgi?id=183010