Skip to content

Commit 341ab62

Browse files
committed
⚡️🔒️ Fix non-linear performance in ResponseReader
A very large response with many small repeated literals can trigger super-linear time. This happens because the regular expression that checks for literal continuation matches from the beginning of the buffer every time. This could be mitigated by searching from an offset, based on what has already been processed, or only searching the most recent line (before merging it with the buffer), but that is still `O(n)` on line length. The regexp is anchored to the end of the string, so searching in reverse from the end of the string should be `O(1)`. This is accomplished by converting `=~` to `rindex`. Note that this _does_ slow down the "no literals" scenario. ``` $ benchmark-driver benchmarks/response_reader.yml --filter KiB Warming up -------------------------------------- 1KiB with no literals 143.564k i/s - 153.197k times in 1.067099s (6.97μs/i) 10KiB with no literals 27.394k i/s - 28.864k times in 1.053670s (36.50μs/i) 100KiB with no literals 2.926k i/s - 3.157k times in 1.079109s (341.81μs/i) 1KiB of 25B literals 2.786k i/s - 2.970k times in 1.066159s (358.98μs/i) 10KiB of 25B literals 263.498 i/s - 286.000 times in 1.085396s (3.80ms/i) 100KiB of 25B literals 19.470 i/s - 20.000 times in 1.027203s (51.36ms/i) 1KiB of 0B literals 530.014 i/s - 530.000 times in 0.999974s (1.89ms/i) 10KiB of 0B literals 45.239 i/s - 50.000 times in 1.105233s (22.10ms/i) 100KiB of 0B literals 3.075 i/s - 4.000 times in 1.300721s (325.18ms/i) Calculating ------------------------------------- local YJIT 1KiB with no literals 137.049k 159.971k i/s - 430.691k times in 3.142607s 2.692304s 10KiB with no literals 27.272k 28.101k i/s - 82.181k times in 3.013413s 2.924470s 100KiB with no literals 2.941k 2.937k i/s - 8.776k times in 2.984095s 2.988129s 1KiB of 25B literals 2.803k 4.136k i/s - 8.357k times in 2.981249s 2.020772s 10KiB of 25B literals 262.978 385.394 i/s - 790.000 times in 3.004055s 2.049850s 100KiB of 25B literals 18.355 22.549 i/s - 58.000 times in 3.159962s 2.572152s 1KiB of 0B literals 505.733 759.572 i/s - 1.590k times in 3.143953s 2.093285s 10KiB of 0B literals 45.414 67.569 i/s - 135.000 times in 2.972648s 1.997962s 100KiB of 0B literals 2.722 3.510 i/s - 9.000 times in 3.306786s 2.564007s Comparison: 1KiB with no literals YJIT: 159971.1 i/s local: 137049.0 i/s - 1.17x slower 10KiB with no literals YJIT: 28101.2 i/s local: 27271.7 i/s - 1.03x slower 100KiB with no literals local: 2940.9 i/s YJIT: 2937.0 i/s - 1.00x slower 1KiB of 25B literals YJIT: 4135.5 i/s local: 2803.2 i/s - 1.48x slower 10KiB of 25B literals YJIT: 385.4 i/s local: 263.0 i/s - 1.47x slower 100KiB of 25B literals YJIT: 22.5 i/s local: 18.4 i/s - 1.23x slower 1KiB of 0B literals YJIT: 759.6 i/s local: 505.7 i/s - 1.50x slower 10KiB of 0B literals YJIT: 67.6 i/s local: 45.4 i/s - 1.49x slower 100KiB of 0B literals YJIT: 3.5 i/s local: 2.7 i/s - 1.29x slower ``` For responses that are larger than 10KiB, the benchmarks do take another dip. Despite that, I believe the algorithm _is_ still linear, and that the performance hit on large responses is probably due to the large strings inducing memory locality (paging/caching) bottlenecks.
1 parent 5a6e3ea commit 341ab62

3 files changed

Lines changed: 24 additions & 2 deletions

File tree

benchmarks/response_reader.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ benchmark:
5858
- { name: "100 B of 0B literals", prelude: "pathological! 1e2", script: "read" }
5959
- { name: " 1KiB of 0B literals", prelude: "pathological! 1e3", script: "read" }
6060
- { name: " 10KiB of 0B literals", prelude: "pathological! 1e4", script: "read" }
61-
# - { name: "100KiB of 0B literals", prelude: "pathological! 1e5", script: "read" }
61+
- { name: "100KiB of 0B literals", prelude: "pathological! 1e5", script: "read" }
6262
# - { name: " 1MiB of 0 byte literals", prelude: "pathological! 1e6", script: "read" }
6363
# - { name: "100MiB of 0 byte literals", prelude: "pathological! 1e8", script: "read" }
6464

lib/net/imap/response_reader.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ def bytes_read = buff.bytesize
3232
def empty? = buff.empty?
3333
def done? = line_done? && !get_literal_size
3434
def line_done? = buff.end_with?(CRLF)
35-
def get_literal_size = /\{(\d+)\}\r\n\z/n =~ buff && $1.to_i
35+
def get_literal_size = buff.rindex(/\{(\d+)\}\r\n\z/n) && $1.to_i
3636

3737
def read_line
3838
buff << (@sock.gets(CRLF, read_limit) or throw :eof)

test/net/imap/test_response_reader.rb

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,4 +82,26 @@ def literal(str) = "{#{str.bytesize}}\r\n#{str}"
8282
end
8383
end
8484

85+
test "linear performance detecting literal continuation" do
86+
omit_unless_cruby "flaky on different platforms"
87+
omit_if(ENV["CI"], "slow and flaky, skipping in CI")
88+
89+
client = FakeClient.new
90+
io = StringIO.new "", "rb"
91+
rcvr = Net::IMAP::ResponseReader.new(client, io)
92+
93+
sequence = [100, 1_000, 10_000]
94+
assert_strict_linear_time(sequence, prepare: ->(n) {
95+
parts = Array.new(n) {|i| "BODY[#{i.succ}] {1}\r\nX" }.join(" ")
96+
response = "* 1 FETCH (#{parts})\r\n"
97+
embedded = "#{response}* OK next response\r\n"
98+
io.string = embedded
99+
assert_equal response, rcvr.read_response_buffer
100+
io.rewind
101+
response
102+
}) do
103+
io.rewind
104+
rcvr.read_response_buffer
105+
end
106+
end
85107
end

0 commit comments

Comments
 (0)