From 67a111a964d59748408a27393255cc11ccc02dfd Mon Sep 17 00:00:00 2001 From: Delyan Angelov Date: Sat, 31 Aug 2024 20:08:14 +0300 Subject: [PATCH 1/4] builtin: fix map.clear() not resetting map's metas and keys blocks (fix #22139) --- vlib/builtin/map.v | 4 ++ vlib/builtin/map_issue_22139_clear_test.v | 59 +++++++++++++++++++++++ 2 files changed, 63 insertions(+) create mode 100644 vlib/builtin/map_issue_22139_clear_test.v diff --git a/vlib/builtin/map.v b/vlib/builtin/map.v index a5a8a0682e98b3..b3fb0d288dbbf2 100644 --- a/vlib/builtin/map.v +++ b/vlib/builtin/map.v @@ -317,6 +317,10 @@ pub fn (mut m map) clear() { m.len = 0 m.even_index = 0 m.key_values.len = 0 + m.key_values.deletes = 0 + unsafe { free(m.key_values.all_deleted) } + unsafe { vmemset(m.key_values.keys, 0, m.key_values.key_bytes * m.key_values.cap) } + unsafe { vmemset(m.metas, 0, sizeof(u32) * (m.even_index + 2 + m.extra_metas)) } } @[inline] diff --git a/vlib/builtin/map_issue_22139_clear_test.v b/vlib/builtin/map_issue_22139_clear_test.v new file mode 100644 index 00000000000000..0c3a5da87d5be0 --- /dev/null +++ b/vlib/builtin/map_issue_22139_clear_test.v @@ -0,0 +1,59 @@ +fn test_map_clear_done_several_times() { + mut ints := map[int]int{} + + ints[5] = 5 + dump(ints.len) + assert ints.len == 1 + + ints.clear() + dump(ints.len) + assert ints.len == 0 + + ints[5] = 3 + dump(ints.len) + assert ints.len == 1 + + ints.clear() + dump(ints.len) + assert ints.len == 0 + + ints[5] = 123 + dump(ints.len) + assert ints.len == 1 +} + +fn test_map_clear_in_loop_metas_should_be_cleared_too() { + mut ints := map[int]int{} + for i in 0 .. 100 { + ints[i] = i * 123 + ints.clear() + assert ints.len == 0 + // dump(ints) + ints[i] = i + // dump(ints) + assert ints.len == 1 + ints[1000 + i] = 1000 * i + assert ints.len == 2 + } +} + +fn test_map_clear_in_loop_delete_keys() { + mut ints := map[int]int{} + for i in 0 .. 100 { + ints[i] = i * 123 + ints[i + 2] = i + ints.delete(i) + ints.delete(i + 1) + ints.delete(i + 2) + ints.delete(i + 3) + ints.delete(i + 4) + ints.clear() + assert ints.len == 0 + // dump(ints) + ints[i] = 5 + // dump(ints) + assert ints.len == 1 + ints[1000 + i] = 1000 * i + assert ints.len == 2 + } +} From 4acbdbd9eaabc5671f5f9e9b9e8dc7e9c98882df Mon Sep 17 00:00:00 2001 From: Delyan Angelov Date: Sat, 31 Aug 2024 20:18:54 +0300 Subject: [PATCH 2/4] fix freeing m.key_values.all_deleted, when there were no deletions, use a single unsafe{} block for clarity --- vlib/builtin/map.v | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/vlib/builtin/map.v b/vlib/builtin/map.v index b3fb0d288dbbf2..2ed6e4de81da42 100644 --- a/vlib/builtin/map.v +++ b/vlib/builtin/map.v @@ -318,9 +318,13 @@ pub fn (mut m map) clear() { m.even_index = 0 m.key_values.len = 0 m.key_values.deletes = 0 - unsafe { free(m.key_values.all_deleted) } - unsafe { vmemset(m.key_values.keys, 0, m.key_values.key_bytes * m.key_values.cap) } - unsafe { vmemset(m.metas, 0, sizeof(u32) * (m.even_index + 2 + m.extra_metas)) } + unsafe { + if m.key_values.all_deleted != 0 { + free(m.key_values.all_deleted) + } + vmemset(m.key_values.keys, 0, m.key_values.key_bytes * m.key_values.cap) + vmemset(m.metas, 0, sizeof(u32) * (m.even_index + 2 + m.extra_metas)) + } } @[inline] From 63a51ff2e42f544fa81bdbf4ff1bf78f979e5466 Mon Sep 17 00:00:00 2001 From: Delyan Angelov Date: Sun, 1 Sep 2024 07:50:07 +0300 Subject: [PATCH 3/4] tools: add bench programs, to check the memory usage after using m.clear() --- cmd/tools/bench/map_clear.v | 17 +++++++++++++++++ cmd/tools/bench/map_clear_runner.vsh | 23 +++++++++++++++++++++++ 2 files changed, 40 insertions(+) create mode 100644 cmd/tools/bench/map_clear.v create mode 100755 cmd/tools/bench/map_clear_runner.vsh diff --git a/cmd/tools/bench/map_clear.v b/cmd/tools/bench/map_clear.v new file mode 100644 index 00000000000000..488b933a5406c2 --- /dev/null +++ b/cmd/tools/bench/map_clear.v @@ -0,0 +1,17 @@ +import benchmark + +max_iterations := arguments()[1] or { '1_000_000' }.int() +assert max_iterations > 0 +mut m := { + 123: 456 + 789: 321 +} +mut volatile sum := u64(0) +mut b := benchmark.start() +for i in 0 .. max_iterations { + m.clear() + m[i] = i * 2 + sum += u64(m.len) +} +assert m.len == 1 +b.measure('m.clear(), iterations: ${max_iterations}, sum: ${sum}') diff --git a/cmd/tools/bench/map_clear_runner.vsh b/cmd/tools/bench/map_clear_runner.vsh new file mode 100755 index 00000000000000..0c82cc82fd1659 --- /dev/null +++ b/cmd/tools/bench/map_clear_runner.vsh @@ -0,0 +1,23 @@ +#!/usr/bin/env -S v -raw-vsh-tmp-prefix tmp + +import os + +const time_fmt = '"CPU: %Us\tReal: %es\tElapsed: %E\tRAM: %MKB\t%C"' +const flags = os.getenv('FLAGS') + +unbuffer_stdout() + +start := os.args[1] or { '1_000_000' }.int() +end := os.args[2] or { '10_000_000' }.int() +step := os.args[3] or { '500_000' }.int() + +os.chdir(os.dir(@VEXE))! +vcmd := 'v ${flags} cmd/tools/bench/map_clear.v' + +println('>> start: ${start} | end: ${end} | step: ${step} | workdir: "${os.getwd()}" | flags: "${flags}" | vcmd: "${vcmd}"') +assert os.system(vcmd) == 0 + +println('running...') +for i := start; i <= end; i += step { + os.system('/usr/bin/time -f ${time_fmt} cmd/tools/bench/map_clear ${i}') == 0 +} From bcbbe105f0bdfd05c284ac8802fa06b751a18cc6 Mon Sep 17 00:00:00 2001 From: Delyan Angelov Date: Sun, 1 Sep 2024 08:13:03 +0300 Subject: [PATCH 4/4] fix vlib/v/parser/v_parser_test.v (which does not expect script programs in .v files) --- cmd/tools/bench/map_clear.v | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/cmd/tools/bench/map_clear.v b/cmd/tools/bench/map_clear.v index 488b933a5406c2..922fc3d1367f94 100644 --- a/cmd/tools/bench/map_clear.v +++ b/cmd/tools/bench/map_clear.v @@ -1,17 +1,19 @@ import benchmark -max_iterations := arguments()[1] or { '1_000_000' }.int() -assert max_iterations > 0 -mut m := { - 123: 456 - 789: 321 +fn main() { + max_iterations := arguments()[1] or { '1_000_000' }.int() + assert max_iterations > 0 + mut m := { + 123: 456 + 789: 321 + } + mut volatile sum := u64(0) + mut b := benchmark.start() + for i in 0 .. max_iterations { + m.clear() + m[i] = i * 2 + sum += u64(m.len) + } + assert m.len == 1 + b.measure('m.clear(), iterations: ${max_iterations}, sum: ${sum}') } -mut volatile sum := u64(0) -mut b := benchmark.start() -for i in 0 .. max_iterations { - m.clear() - m[i] = i * 2 - sum += u64(m.len) -} -assert m.len == 1 -b.measure('m.clear(), iterations: ${max_iterations}, sum: ${sum}')