8000 Avoid rbuf copying on the first read by marshall-lee · Pull Request #16 · ruby/net-protocol · GitHub
[go: up one dir, main page]
Skip to content

Avoid rbuf copying on the first read #16

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Avoid rbuf copying on the first read #16

wants to merge 2 commits into from

Conversation

marshall-lee
Copy link

When the caller doesn't specify the destination, there's no need to append the rbuf to empty string - the rbuf slice itself could be used as initial dest value.

@marshall-lee
Avoid rbuf copying on the first read
4eb5ab0 
When the caller doesn't specify the destination, there's no need
to append the rbuf to empty string - the rbuf slice itself could
be used as initial dest value.
@marshall-lee
Reset the read buffer when len exceeds its size
69cb5ea
@marshall-lee
Copy link
Author

Ping @byroot

@casperisfine
Copy link

Indeed, but I don't think it makes a noticeable difference in performance. I adapter my benchmark from #15: https://gist.github.com/casperisfine/1d6be6822c27f8e4ad91a9296f1eb01f

(your implementation is opt2):

=== 1k ===
Warming up --------------------------------------
                  1k     1.207k i/100ms
              1k opt     1.204k i/100ms
             1k opt2     1.210k i/100ms
Calculating -------------------------------------
                  1k     11.826k (± 2.1%) i/s -     59.143k in   5.003186s
              1k opt     12.043k (± 4.0%) i/s -     60.200k in   5.008535s
             1k opt2     11.999k (± 4.8%) i/s -     60.500k in   5.055999s

Comparison:
                  1k:    11826.4 i/s
              1k opt:    12042.8 i/s - same-ish: difference falls within error
             1k opt2:    11999.2 i/s - same-ish: difference falls within error


=== 10k ===
Warming up --------------------------------------
                 10k     1.092k i/100ms
             10k opt     1.191k i/100ms
            10k opt2     1.065k i/100ms
Calculating -------------------------------------
                 10k     10.834k (± 2.2%) i/s -     54.600k in   5.042292s
             10k opt     11.895k (± 2.0%) i/s -     59.550k in   5.008356s
            10k opt2     11.785k (± 2.6%) i/s -     59.640k in   5.064518s

Comparison:
                 10k:    10833.9 i/s
             10k opt:    11895.0 i/s - 1.10x  (± 0.00) faster
            10k opt2:    11784.8 i/s - 1.09x  (± 0.00) faster


=== 100k ===
Warming up --------------------------------------
                100k   707.000  i/100ms
            100k opt   807.000  i/100ms
           100k opt2   798.000  i/100ms
Calculating -------------------------------------
                100k      6.868k (± 4.4%) i/s -     34.643k in   5.055588s
            100k opt      7.986k (± 1.4%) i/s -     40.350k in   5.053437s
           100k opt2      7.970k (± 1.7%) i/s -     39.900k in   5.007612s

Comparison:
                100k:     6868.1 i/s
            100k opt:     7986.3 i/s - 1.16x  (± 0.00) faster
           100k opt2:     7970.2 i/s - 1.16x  (± 0.00) faster


=== 1M ===
Warming up --------------------------------------
                  1M   194.000  i/100ms
              1M opt   279.000  i/100ms
             1M opt2   278.000  i/100ms
Calculating -------------------------------------
                  1M      2.063k (± 9.1%) i/s -     10.282k in   5.041130s
              1M opt      2.641k (± 3.7%) i/s -     13.392k in   5.077949s
             1M opt2      2.660k (± 4.3%) i/s -     13.344k in   5.027997s

Comparison:
                  1M:     2063.4 i/s
             1M opt2:     2659.6 i/s - 1.29x  (± 0.00) faster
              1M opt:     2640.9 i/s - 1.28x  (± 0.00) faster

@marshall-lee
Copy link
Author

@casperisfine the benchmark doesn't follow this path because Net::HTTP allocates its own buffer https://github.com/ruby/net-http/blob/3237ef4d8c43c59cbfb477476fb942f8467ab930/lib/net/http/response.rb#L510-L518

@casperisfine
Copy link

Sure, but then in which case if this change useful? Are you using net-protocol for something else?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@marshall-lee @casperisfine
0