Skip to content

Commit

Permalink
Merge branch 'maint'
Browse files Browse the repository at this point in the history
* maint:
  Fix heap allocation for matching out short bitstrings
  • Loading branch information
bjorng committed Aug 25, 2023
2 parents 7f9f8f7 + 8d41073 commit 75de35f
Show file tree
Hide file tree
Showing 6 changed files with 68 additions and 17 deletions.
2 changes: 1 addition & 1 deletion erts/emulator/beam/emu/generators.tab
Original file line number Diff line number Diff line change
Expand Up @@ -1100,7 +1100,7 @@ gen.bs_match(Fail, Ctx, N, List) {
ASSERT(List[src+3].type == TAG_u);
ASSERT(List[src+4].type == TAG_u);
size = List[src+3].val * List[src+4].val;
words_needed = heap_bin_size((size + 7) / 8);
words_needed = erts_extracted_binary_size(size);
break;
case am_integer:
ASSERT(List[src+3].type == TAG_u);
Expand Down
16 changes: 16 additions & 0 deletions erts/emulator/beam/erl_bits.c
Original file line number Diff line number Diff line change
Expand Up @@ -2345,6 +2345,22 @@ erts_copy_bits(const byte* src, /* Base pointer to source. */
}
}

/*
* Calculate sufficient heap space for a binary extracted by
* erts_extract_sub_binary().
*/
Uint erts_extracted_binary_size(Uint bit_size)
{
Uint byte_size = BYTE_OFFSET(bit_size);
ERTS_CT_ASSERT(ERL_SUB_BIN_SIZE <= ERL_ONHEAP_BIN_LIMIT);

if (BIT_OFFSET(bit_size) == 0 && byte_size <= ERL_ONHEAP_BIN_LIMIT) {
return heap_bin_size(byte_size);
} else {
return ERL_SUB_BIN_SIZE;
}
}

Eterm erts_extract_sub_binary(Eterm **hp, Eterm base_bin, byte *base_data,
Uint bit_offset, Uint bit_size)
{
Expand Down
17 changes: 14 additions & 3 deletions erts/emulator/beam/erl_bits.h
Original file line number Diff line number Diff line change
Expand Up @@ -184,16 +184,27 @@ void erts_copy_bits(const byte* src, size_t soffs, int sdir,
byte* dst, size_t doffs, int ddir, size_t n);
int erts_cmp_bits(byte* a_ptr, size_t a_offs, byte* b_ptr, size_t b_offs, size_t size);

/*
* Calculate the heap space for a binary extracted by
* erts_extract_sub_binary().
*/
Uint erts_extracted_binary_size(Uint bit_size);

/* Extracts a region from base_bin as a sub-binary or heap binary, whichever
* is the most appropriate.
*
* The caller must ensure that there's enough free space at *hp */
* The caller must ensure that there's enough free space at *hp by using
* erts_extracted_binary_size().
* */
Eterm erts_extract_sub_binary(Eterm **hp, Eterm base_bin, byte *base_data,
Uint bit_offset, Uint num_bits);

/* Pessimistic estimate of the words required for erts_extract_sub_binary */
#define EXTRACT_SUB_BIN_HEAP_NEED (heap_bin_size(ERL_ONHEAP_BIN_LIMIT))
/*
* Conservative estimate of the number of words required for
* erts_extract_sub_binary() when the number of bits is unknown.
*/
#define EXTRACT_SUB_BIN_HEAP_NEED \
(MAX(ERL_SUB_BIN_SIZE, heap_bin_size(ERL_ONHEAP_BIN_LIMIT)))

/*
* Flags for bs_create_bin / bs_get_* / bs_put_* / bs_init* instructions.
Expand Down
2 changes: 1 addition & 1 deletion erts/emulator/beam/jit/arm/instr_bs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3706,7 +3706,7 @@ static std::vector<BsmSegment> opt_bsm_segments(
}
break;
case BsmSegment::action::GET_BINARY:
heap_need += heap_bin_size((seg.size + 7) / 8);
heap_need += erts_extracted_binary_size(seg.size);
break;
case BsmSegment::action::GET_TAIL:
heap_need += EXTRACT_SUB_BIN_HEAP_NEED;
Expand Down
2 changes: 1 addition & 1 deletion erts/emulator/beam/jit/x86/instr_bs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3829,7 +3829,7 @@ static std::vector<BsmSegment> opt_bsm_segments(
}
break;
case BsmSegment::action::GET_BINARY:
heap_need += heap_bin_size((seg.size + 7) / 8);
heap_need += erts_extracted_binary_size(seg.size);
break;
case BsmSegment::action::GET_TAIL:
heap_need += EXTRACT_SUB_BIN_HEAP_NEED;
Expand Down
46 changes: 35 additions & 11 deletions erts/emulator/test/bs_match_bin_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,16 @@
init_per_group/2,end_per_group/2,
byte_split_binary/1,bit_split_binary/1,match_huge_bin/1,
bs_match_string_edge_case/1,contexts/1,
empty_binary/1]).
empty_binary/1,small_bitstring/1]).

-include_lib("common_test/include/ct.hrl").

suite() -> [{ct_hooks,[ts_install_cth]}].

all() ->
[byte_split_binary, bit_split_binary, match_huge_bin,
bs_match_string_edge_case, contexts, empty_binary].
bs_match_string_edge_case, contexts, empty_binary,
small_bitstring].

groups() ->
[].
Expand Down Expand Up @@ -113,14 +114,6 @@ bits_to_list([], _) -> [].

mkbin(L) when is_list(L) -> list_to_binary(L).

make_unaligned_sub_binary(Bin0) ->
Bin1 = <<0:3,Bin0/binary,31:5>>,
Sz = size(Bin0),
<<0:3,Bin:Sz/binary,31:5>> = id(Bin1),
Bin.

id(I) -> I.

match_huge_bin(Config) when is_list(Config) ->
Bin = <<0:(1 bsl 27),13:8>>,
skip_huge_bin_1(1 bsl 27, Bin),
Expand Down Expand Up @@ -280,6 +273,37 @@ do_empty_binary(0) ->
do_empty_binary(N) ->
%% The new bs_match instruction would use more heap space
%% than reserved when matching out an empty binary.
<<V1:0/bits, V1:0/bitstring, V2:0/bytes, V2:0/bits>> = <<>>,
<<V1:0/bits, V1:0/bitstring, V2:0/bytes, V2:0/bits>> = id(<<>>),
[0|do_empty_binary(N-1)].

small_bitstring(_Config) ->
%% GH-7292: The new bs_match instruction would reserve insufficient
%% heap space for small bitstrings.
rand_seed(),
Bin = rand:bytes(10_000),
ok = small_bitstring_1(id(Bin), id(Bin)).

small_bitstring_1(<<A1:1/bits,A2:1/bits,A3:2/bits,
A4:3/bits,A5:1/bits,As0/binary>>,
<<A1:1/bits,A2:1/bits,A3:2/bits,
A4:3/bits,A5:1/bits,As1/binary>>) ->
small_bitstring_1(As0, As1);
small_bitstring_1(<<>>, <<>>) ->
ok.

%%%
%%% Common utilities.
%%%

rand_seed() ->
rand:seed(default),
io:format("\n*** rand:export_seed() = ~w\n\n", [rand:export_seed()]),
ok.

make_unaligned_sub_binary(Bin0) ->
Bin1 = <<0:3,Bin0/binary,31:5>>,
Sz = size(Bin0),
<<0:3,Bin:Sz/binary,31:5>> = id(Bin1),
Bin.

id(I) -> I.

0 comments on commit 75de35f

Please sign in to comment.