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

about func (c *Client) newResponse(res *http.Response) #622

Closed
zplzpl opened this issue Oct 17, 2017 · 5 comments
Closed

about func (c *Client) newResponse(res *http.Response) #622

zplzpl opened this issue Oct 17, 2017 · 5 comments

Comments

@zplzpl
Copy link

zplzpl commented Oct 17, 2017

Please use the following questions as a guideline to help me answer
your issue/question without further inquiry. Thank you.

Which version of Elastic are you using?

[*] elastic.v5 (for Elasticsearch 5.x)

json package:
type RawMessage []byte

elsatic package:
`// Response represents a response from Elasticsearch.
type Response struct {
// StatusCode is the HTTP status code, e.g. 200.
StatusCode int
// Header is the HTTP header from the HTTP response.
// Keys in the map are canonicalized (see http.CanonicalHeaderKey).
Header http.Header
// Body is the deserialized response body.
Body json.RawMessage
}

// newResponse creates a new response from the HTTP response.
func (c *Client) newResponse(res *http.Response) (*Response, error) {
r := &Response{
StatusCode: res.StatusCode,
Header: res.Header,
}
if res.Body != nil {
slurp, err := ioutil.ReadAll(res.Body)
if err != nil {
return nil, err
}
// HEAD requests return a body but no content
if len(slurp) > 0 {
if err := c.decoder.Decode(slurp, &r.Body); err != nil {
return nil, err
}
}
}
return r, nil
}`

slurp data type has been [] byte, why do you need to decode for *json.RawMessage???

@olivere
Copy link
Owner

olivere commented Oct 17, 2017

Good point, thank you. On the first tests, this is a clear win, reducing JSON serialization time from ~1500 ns/op to ~1000 ns/op. Amazing!

See a220ed3

olivere added a commit that referenced this issue Oct 17, 2017
olivere added a commit that referenced this issue Oct 26, 2017
@olivere
Copy link
Owner

olivere commented Oct 26, 2017

Thanks a lot. This will be part of 5.0.53, which will be released soon. 👍

@olivere olivere closed this as completed Oct 26, 2017
@agbetz64
Copy link

agbetz64 commented Nov 2, 2017

Have you ever considered/tested jsoniter?

@olivere
Copy link
Owner

olivere commented Nov 3, 2017

@agbetz64 No. I want to stay close to the standard library. But you can use your own Decoder if you like.

@olivere
Copy link
Owner

olivere commented Jan 8, 2018

@agbetz64 @zplzpl There is performance-related work on the way in #667 to increase performance for at least Bulk API. If you have time to support/review/test, that'd be appreciated.

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

No branches or pull requests

3 participants