Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

client: URI is always parsed #1141

Closed
stokito opened this issue Oct 29, 2021 · 2 comments
Closed

client: URI is always parsed #1141

stokito opened this issue Oct 29, 2021 · 2 comments

Comments

@stokito
Copy link
Contributor

stokito commented Oct 29, 2021

To perform an HTTP requests each time the SetRequestURIBytes() must be called. But internally the method just saves an URL and it is then parsed inside of parseURI():

func (req *Request) parseURI() error {
	if req.parsedURI {
		return nil
	}
	req.parsedURI = true
	return req.uri.parse(req.Header.Host(), req.Header.RequestURI(), req.isTLS)
}

My application has a bunch of fixed URLs like http://example.com/api/ and I can call the req.uri.parse() only once on initialization. And then I would like to just set this fasthttp.URI objects before sending the request with a new method SetRequestURI(preparsedUri).

Is any reason why this wasn't implemented? Will you implement this or accept a PR with such functionality?

@erikdubbelboer
Copy link
Collaborator

To be honest I'm not sure why it was implemented like that. Yes I would accept a PR to support such functionality.

stokito added a commit to stokito/fasthttp that referenced this issue Nov 7, 2021
Currently, the only way to set URI for a request is to call SetRequestURI(string).
Then when a request performed the string will be parsed into a fasthttp.URI struct.
If there are many requests with the same URI then we'll waste CPU for a parsing of the same URI string.
With the new SetURI(*URI) method we can once parse a URI string into a fasthttp.URI struct and then reuse it for many requests.
Unfortunately the URI will be copied because may be modified inside the request.
But anyway this will be more lightweight than parsing.
@stokito
Copy link
Contributor Author

stokito commented Nov 10, 2021

Thank you for merging, this should be useful if we have a base URI that can be copied and some part can be changed.
But I found even a better way for my case when API endpoint is static:

  1. Create a separate request pool for each API endpoint
  2. Create a request manually and set URI and headers
  3. Execute the request
  4. Put the request back to pool but without calling the Reset()
  5. Next time when the request will be taken from the pool it will already have a parsed URI and headers. You just need to override a body
var requestPool sync.Pool

func main() {
        reqBody := []byte("{\"id\": 1}")
        // AcquireRequest
	reqV := requestPool.Get()
	var req *fasthttp.Request
	if reqV != nil {
		req = reqV.(*fasthttp.Request)
	} else {
		req = &fasthttp.Request{}
		req.SetRequestURIBytes("http://example.com/api/v1/")
		req.Header.SetMethod(fasthttp.MethodPost)
		req.Header.SetContentType("application/json")
	}
	resp := fasthttp.AcquireResponse()
	req.SetBodyRaw(reqBody)
	err := fasthttp.Do(req, resp)
	// Same as fasthttp.ReleaseRequest but without reset
	requestPool.Put(req)
	fasthttp.ReleaseResponse(resp)
}

I hope this solution may be useful for someone. Interesting if there is any way to make it easier to use.
Maybe we remove the `Reset()` on release and leave this for users. Also we may create a separate type for pools so users may create their own instead of using a global pool.
This is just some thoughts and I don't think that fasthhtp should be changed for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants