On March 25, 2025 6:53 pm, Thomas Lamprecht wrote: > Am 25.03.25 um 16:12 schrieb Daniel Kral: >> Implement helper subroutines, which implement basic set operations done >> on hash sets, i.e. hashes with elements set to a true value, e.g. 1. >> >> These will be used for various tasks in the HA Manager colocation rules, >> e.g. for verifying the satisfiability of the rules or applying the >> colocation rules on the allowed set of nodes. >> >> Signed-off-by: Daniel Kral <d.k...@proxmox.com> >> --- >> If they're useful somewhere else, I can move them to PVE::Tools >> post-RFC, but it'd be probably useful to prefix them with `hash_` there. > > meh, not a big fan of growing the overly generic PVE::Tools more, if, this > should go into a dedicated module for hash/data structure helpers ... > >> AFAICS there weren't any other helpers for this with a quick grep over >> all projects and `PVE::Tools::array_intersect()` wasn't what I needed. > > ... which those existing one should then also move into, but out of scope > of this series. > >> >> src/PVE/HA/Tools.pm | 42 ++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 42 insertions(+) >> >> diff --git a/src/PVE/HA/Tools.pm b/src/PVE/HA/Tools.pm >> index 0f9e9a5..fc3282c 100644 >> --- a/src/PVE/HA/Tools.pm >> +++ b/src/PVE/HA/Tools.pm >> @@ -115,6 +115,48 @@ sub write_json_to_file { >> PVE::Tools::file_set_contents($filename, $raw); >> } >> >> +sub is_disjoint { > > IMO a bit too generic name for being in a Tools named module, maybe > prefix them all with hash_ or hashes_ ?
is_disjoint also only really makes sense as a name if you see it as an operation *on* $hash1, rather than an operation involving both hashes.. i.e., in Rust set1.is_disjoint(&set2); makes sense.. in Perl is_disjoint($set1, $set2) reads weird, and should maybe be check_disjoint($set1, $set2) or something like that? > >> + my ($hash1, $hash2) = @_; >> + >> + for my $key (keys %$hash1) { >> + return 0 if exists($hash2->{$key}); >> + } >> + >> + return 1; >> +}; >> + >> +sub intersect { >> + my ($hash1, $hash2) = @_; >> + >> + my $result = { map { $_ => $hash2->{$_} } keys %$hash1 }; this is a bit dangerous if $hash2->{$key} is itself a reference? if I later modify $result I'll modify $hash2.. I know the commit message says that the hashes are all just of the form key => 1, but nothing here tells me that a year later when I am looking for a generic hash intersection helper ;) I think this should also be clearly mentioned in the module, and ideally, also in the helper names (i.e., have "set" there everywhere and a comment above each that it only works for hashes-as-sets and not generic hashes). wouldn't it be faster/simpler to iterate over either hash once? my $result = {}; for my $key (keys %$hash1) { $result->{$key} = 1 if $hash1->{$key} && $hash2->{$key}; } return $result; >> + >> + for my $key (keys %$result) { >> + delete $result->{$key} if !defined($result->{$key}); >> + } >> + >> + return $result; >> +}; >> + >> +sub set_difference { >> + my ($hash1, $hash2) = @_; >> + >> + my $result = { map { $_ => 1 } keys %$hash1 }; if $hash1 is only of the form key => 1, then this is just my $result = { %$hash1 }; >> + >> + for my $key (keys %$result) { >> + delete $result->{$key} if defined($hash2->{$key}); >> + } >> + but the whole thing can be return { map { $hash2->{$_} ? ($_ => 1) : () } keys %$hash1 }; this transforms hash1 into its keys, and then returns either ($key => 1) if the key is true in $hash2, or the empty tuple if not. the outer {} then turn this sequence of tuples into a hash again, which skips empty tuples ;) can of course also be adapted to use the value from either hash, check for definedness instead of truthiness, .. >> + return $result; >> +}; >> + >> +sub union { >> + my ($hash1, $hash2) = @_; >> + >> + my $result = { map { $_ => 1 } keys %$hash1, keys %$hash2 }; >> + >> + return $result; >> +}; >> + >> sub count_fenced_services { >> my ($ss, $node) = @_; >> > > > > _______________________________________________ > pve-devel mailing list > pve-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel > > > _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel