From 6d034a1baa054d0c0a85b7c7eed7a9ccae52450e Mon Sep 17 00:00:00 2001 From: Mat Trudel Date: Thu, 19 Dec 2024 17:04:36 -0500 Subject: [PATCH 1/3] evict_to_size should only evict if it needs to (Previously, it would *always* evict at least one record even if size was <= new_size) --- lib/hpax/table.ex | 2 ++ test/hpax/table_test.exs | 37 ++++++++++++++++++++++++++++--------- 2 files changed, 30 insertions(+), 9 deletions(-) diff --git a/lib/hpax/table.ex b/lib/hpax/table.ex index 8ca6587..b494bf8 100644 --- a/lib/hpax/table.ex +++ b/lib/hpax/table.ex @@ -317,6 +317,8 @@ defmodule HPAX.Table do # Removes records as necessary to have the total size of entries within the table be less than # or equal to the specified value. Does not change the table's max size. + defp evict_to_size(%__MODULE__{size: size} = table, new_size) when size <= new_size, do: table + defp evict_to_size(%__MODULE__{entries: entries, size: size} = table, new_size) do {new_entries_reversed, new_size} = evict_towards_size(Enum.reverse(entries), size, new_size) diff --git a/test/hpax/table_test.exs b/test/hpax/table_test.exs index a73a4c0..09618b8 100644 --- a/test/hpax/table_test.exs +++ b/test/hpax/table_test.exs @@ -70,7 +70,7 @@ defmodule HPAX.TableTest do end end - describe "resizing" do + describe "resize/2" do test "increasing the protocol max table size" do table = Table.new(4096, :never) table = Table.add(table, "aaaa", "AAAA") @@ -80,25 +80,44 @@ defmodule HPAX.TableTest do assert table.protocol_max_table_size == 8192 end - test "decreasing the protocol max table size not below the max table size" do + test "decreasing the protocol max table size but above table size" do + table = Table.new(4096, :never) + table = Table.add(table, "aaaa", "AAAA") + table = Table.resize(table, 2048) + assert table.size == 40 + assert table.max_table_size == 2048 + assert table.protocol_max_table_size == 2048 + end + + test "decreasing the protocol max table size below current size should evict" do table = Table.new(4096, :never) table = Table.add(table, "aaaa", "AAAA") table = Table.add(table, "bbbb", "BBBB") + table = Table.resize(table, 60) + assert table.size == 40 + assert table.max_table_size == 60 + assert table.protocol_max_table_size == 60 + end + end + + describe "dynamic_resize/2" do + test "decreasing the max table size but above table size" do + table = Table.new(4096, :never) + table = Table.add(table, "aaaa", "AAAA") table = Table.dynamic_resize(table, 2048) - table = Table.resize(table, 6000) assert table.size == 40 - assert table.max_table_size == 6000 - assert table.protocol_max_table_size == 6000 + assert table.max_table_size == 2048 + assert table.protocol_max_table_size == 4096 end - test "decreasing the protocol max table size below the max table size" do + test "decreasing the protocol max table size below current size should evict" do table = Table.new(4096, :never) table = Table.add(table, "aaaa", "AAAA") table = Table.add(table, "bbbb", "BBBB") - table = Table.resize(table, 40) + table = Table.dynamic_resize(table, 60) assert table.size == 40 - assert table.max_table_size == 40 - assert table.protocol_max_table_size == 40 + assert table.max_table_size == 60 + assert table.protocol_max_table_size == 4096 end end end From e82d8e1b9432fc22bb301e3021e6b65d33a4d818 Mon Sep 17 00:00:00 2001 From: Mat Trudel Date: Thu, 19 Dec 2024 17:10:16 -0500 Subject: [PATCH 2/3] Ensure we *always* send a dynamic table resize command MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closer reading of RFC9113§4.3.1para¶4 (https://www.rfc-editor.org/rfc/rfc9113.html#section-4.3.1) suggests that we should always send a dynamic resize Fixes #20 --- lib/hpax/table.ex | 18 ++++++++---------- test/hpax_test.exs | 38 ++++++++++++++++++++++++++++++++++---- 2 files changed, 42 insertions(+), 14 deletions(-) diff --git a/lib/hpax/table.ex b/lib/hpax/table.ex index b494bf8..c1e746d 100644 --- a/lib/hpax/table.ex +++ b/lib/hpax/table.ex @@ -264,15 +264,6 @@ defmodule HPAX.Table do In all cases, the table's `:protocol_max_table_size` is updated accordingly """ @spec resize(t(), non_neg_integer()) :: t() - def resize(%__MODULE__{max_table_size: max_table_size} = table, new_protocol_max_table_size) - when new_protocol_max_table_size >= max_table_size do - %__MODULE__{ - table - | protocol_max_table_size: new_protocol_max_table_size, - max_table_size: new_protocol_max_table_size - } - end - def resize(%__MODULE__{} = table, new_protocol_max_table_size) do pending_minimum_resize = case table.pending_minimum_resize do @@ -280,8 +271,15 @@ defmodule HPAX.Table do current -> min(current, new_protocol_max_table_size) end + table = + if new_protocol_max_table_size < table.size do + evict_to_size(table, new_protocol_max_table_size) + else + table + end + %__MODULE__{ - evict_to_size(table, new_protocol_max_table_size) + table | protocol_max_table_size: new_protocol_max_table_size, max_table_size: new_protocol_max_table_size, pending_minimum_resize: pending_minimum_resize diff --git a/test/hpax_test.exs b/test/hpax_test.exs index 1510b36..72bcc68 100644 --- a/test/hpax_test.exs +++ b/test/hpax_test.exs @@ -109,7 +109,6 @@ defmodule HPAXTest do property "encode/3 prepends dynamic resizes at the start of a block" do enc_table = HPAX.new(20_000) - # Start with a non-empty decode table dec_table = HPAX.new(20_000) # Put a record in both to prime the pump. The table sizes should match @@ -120,14 +119,45 @@ defmodule HPAXTest do assert enc_table.max_table_size == 20_000 assert dec_table.max_table_size == 20_000 - # Encode a record after resizing the table. We expect a dynamic resize to be - # encoded and the for two table sizes to be identical after decoding + # Scenario 1: Simulate the decoder growing the table via settings + + # First, the decoder resizes its table to some maximum size + dec_table = HPAX.resize(dec_table, 40_000) + + # It then communicates that size to the encoder, who chooses a smaller size + enc_table = HPAX.resize(enc_table, 30_000) + + # Now, encode a header + {encoded, enc_table} = HPAX.encode([{:store, "lame", "LAME"}], enc_table) + encoded = IO.iodata_to_binary(encoded) + + # Ensure that we encoded a resize on the wire + assert <<0b001::3, rest::bitstring>> = encoded + assert {:ok, 30_000, _rest} = HPAX.Types.decode_integer(rest, 5) + + # Finally, ensure that the decoder makes proper sense of this encoding and that it resizes + # back down to the size chosen by the encoder + assert {:ok, _decoded, dec_table} = HPAX.decode(encoded, dec_table) + assert dec_table.size == enc_table.size + assert enc_table.max_table_size == 30_000 + assert dec_table.max_table_size == 30_000 + + # Scenario 2: Simulate the decoder shrinking the table ia settings + + # First, the decoder resizes its table to some maximum size + dec_table = HPAX.resize(dec_table, 10_000) + + # It then communicates that size to the encoder, who chooses a smaller size enc_table = HPAX.resize(enc_table, 0) + + # It then changes its mind and goes back up to a size still smaller than the decoder's choice enc_table = HPAX.resize(enc_table, 1234) + + # Now, encode a header {encoded, enc_table} = HPAX.encode([{:store, "lame", "LAME"}], enc_table) encoded = IO.iodata_to_binary(encoded) - # Ensure that we see two resizes in order + # Ensure that we encoded two resizes in order on the wire assert <<0b001::3, rest::bitstring>> = encoded assert {:ok, 0, rest} = HPAX.Types.decode_integer(rest, 5) assert <<0b001::3, rest::bitstring>> = rest From d892f265ddeb6a5729eda4acb9802f149f4728fd Mon Sep 17 00:00:00 2001 From: Mat Trudel Date: Fri, 20 Dec 2024 09:14:29 -0500 Subject: [PATCH 3/3] PR comments --- lib/hpax/table.ex | 9 +-------- test/hpax/table_test.exs | 4 ++-- 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/lib/hpax/table.ex b/lib/hpax/table.ex index c1e746d..b82763c 100644 --- a/lib/hpax/table.ex +++ b/lib/hpax/table.ex @@ -271,15 +271,8 @@ defmodule HPAX.Table do current -> min(current, new_protocol_max_table_size) end - table = - if new_protocol_max_table_size < table.size do - evict_to_size(table, new_protocol_max_table_size) - else - table - end - %__MODULE__{ - table + evict_to_size(table, new_protocol_max_table_size) | protocol_max_table_size: new_protocol_max_table_size, max_table_size: new_protocol_max_table_size, pending_minimum_resize: pending_minimum_resize diff --git a/test/hpax/table_test.exs b/test/hpax/table_test.exs index 09618b8..f7cfd0d 100644 --- a/test/hpax/table_test.exs +++ b/test/hpax/table_test.exs @@ -70,7 +70,7 @@ defmodule HPAX.TableTest do end end - describe "resize/2" do + describe "resizing" do test "increasing the protocol max table size" do table = Table.new(4096, :never) table = Table.add(table, "aaaa", "AAAA") @@ -100,7 +100,7 @@ defmodule HPAX.TableTest do end end - describe "dynamic_resize/2" do + describe "dynamically resizing" do test "decreasing the max table size but above table size" do table = Table.new(4096, :never) table = Table.add(table, "aaaa", "AAAA")