From 4f58ae60eb781b9ade164c4ea2abad708d00f4ce Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Thu, 2 Jun 2022 10:13:55 -0600 Subject: [PATCH 01/23] [map] Keep is_used, is_tombstone as booleans --- src/hb-map.hh | 44 ++++++++++++++++++++------------------------ 1 file changed, 20 insertions(+), 24 deletions(-) diff --git a/src/hb-map.hh b/src/hb-map.hh index 3084d478d..3df31ed98 100644 --- a/src/hb-map.hh +++ b/src/hb-map.hh @@ -71,6 +71,16 @@ struct hb_hashmap_t uint32_t hash; V value; + bool is_used_; + bool is_tombstone_; + + + bool is_used () const { return is_used_; } + void set_used (bool is_used) { is_used_ = is_used; } + bool is_tombstone () const { return is_tombstone_; } + void set_tombstone (bool is_tombstone) { is_tombstone_ = is_tombstone; } + bool is_real () const { return is_used_ && !is_tombstone_; } + void clear () { new (std::addressof (key)) K (); @@ -78,27 +88,12 @@ struct hb_hashmap_t new (std::addressof (value)) V (); value = hb_coerce (vINVALID); hash = 0; + is_used_ = false; + is_tombstone_ = false; } bool operator == (const K &o) { return hb_deref (key) == hb_deref (o); } bool operator == (const item_t &o) { return *this == o.key; } - bool is_unused () const - { - const K inv = hb_coerce (kINVALID); - return key == inv; - } - bool is_tombstone () const - { - const K kinv = hb_coerce (kINVALID); - const V vinv = hb_coerce (vINVALID); - return key != kinv && value == vinv; - } - bool is_real () const - { - const K kinv = hb_coerce (kINVALID); - const V vinv = hb_coerce (vINVALID); - return key != kinv && value != vinv; - } hb_pair_t get_pair() const { return hb_pair_t (key, value); } uint32_t total_hash () const @@ -213,7 +208,7 @@ struct hb_hashmap_t return items[i].is_real () && items[i] == key ? items[i].value : hb_coerce (vINVALID); } - void del (K key) { set (key, hb_coerce (vINVALID)); } + void del (K key) { set_with_hash (key, hb_hash (key), hb_coerce (vINVALID), true); } /* Has interface. */ typedef V value_t; @@ -297,7 +292,7 @@ struct hb_hashmap_t protected: template - bool set_with_hash (K key, uint32_t hash, VV&& value) + bool set_with_hash (K key, uint32_t hash, VV&& value, bool is_delete=false) { if (unlikely (!successful)) return false; const K kinv = hb_coerce (kINVALID); @@ -305,11 +300,10 @@ struct hb_hashmap_t if (unlikely ((occupancy + occupancy / 2) >= mask && !resize ())) return false; unsigned int i = bucket_for_hash (key, hash); - const V vinv = hb_coerce (vINVALID); - if (value == vinv && items[i].key != key) + if (is_delete && items[i].key != key) return true; /* Trying to delete non-existent key. */ - if (!items[i].is_unused ()) + if (items[i].is_used ()) { occupancy--; if (!items[i].is_tombstone ()) @@ -319,9 +313,11 @@ struct hb_hashmap_t items[i].key = key; items[i].value = value; items[i].hash = hash; + items[i].set_used (true); + items[i].set_tombstone (is_delete); occupancy++; - if (!items[i].is_tombstone ()) + if (!is_delete) population++; return true; @@ -337,7 +333,7 @@ struct hb_hashmap_t unsigned int i = hash % prime; unsigned int step = 0; unsigned int tombstone = (unsigned) -1; - while (!items[i].is_unused ()) + while (items[i].is_used ()) { if (items[i].hash == hash && items[i] == key) return i; From 5328b73fbaf5b952cf7eb23a7b4a228585502c10 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Thu, 2 Jun 2022 10:32:56 -0600 Subject: [PATCH 02/23] [map] Reduce map item size again --- src/hb-map.hh | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/hb-map.hh b/src/hb-map.hh index 3df31ed98..7eb823e5e 100644 --- a/src/hb-map.hh +++ b/src/hb-map.hh @@ -68,11 +68,11 @@ struct hb_hashmap_t struct item_t { K key; - uint32_t hash; + uint32_t hash : 30; + uint32_t is_used_ : 1; + uint32_t is_tombstone_ : 1; V value; - bool is_used_; - bool is_tombstone_; bool is_used () const { return is_used_; } @@ -330,6 +330,7 @@ struct hb_hashmap_t unsigned int bucket_for_hash (K key, uint32_t hash) const { + hash &= 0x3FFFFFFF; // We only store lower 30bit of hash unsigned int i = hash % prime; unsigned int step = 0; unsigned int tombstone = (unsigned) -1; From 3f6a8f69a099398ac397bb652e6d8332167f6538 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Thu, 2 Jun 2022 10:36:07 -0600 Subject: [PATCH 03/23] [map] Remove invalid-key special-casing Can override invalid-key value now. --- src/hb-map.hh | 3 --- src/test-map.cc | 15 ++++++--------- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/src/hb-map.hh b/src/hb-map.hh index 7eb823e5e..a0ab17a23 100644 --- a/src/hb-map.hh +++ b/src/hb-map.hh @@ -84,7 +84,6 @@ struct hb_hashmap_t void clear () { new (std::addressof (key)) K (); - key = hb_coerce (kINVALID); new (std::addressof (value)) V (); value = hb_coerce (vINVALID); hash = 0; @@ -295,8 +294,6 @@ struct hb_hashmap_t bool set_with_hash (K key, uint32_t hash, VV&& value, bool is_delete=false) { if (unlikely (!successful)) return false; - const K kinv = hb_coerce (kINVALID); - if (unlikely (key == kinv)) return true; if (unlikely ((occupancy + occupancy / 2) >= mask && !resize ())) return false; unsigned int i = bucket_for_hash (key, hash); diff --git a/src/test-map.cc b/src/test-map.cc index 3041195a6..a5e85074c 100644 --- a/src/test-map.cc +++ b/src/test-map.cc @@ -168,9 +168,8 @@ main (int argc, char **argv) m1.set (hb_map_t {pair (1u, 2u)}, hb_map_t {pair (2u, 3u)}); m2.set (hb_map_t {pair (1u, 2u)}, hb_map_t {pair (2u, 3u)}); - /* Cannot override empty map. */ - assert (m1.get (hb_map_t ()) == hb_map_t ()); - assert (m2.get (hb_map_t ()) == hb_map_t ()); + assert (m1.get (hb_map_t ()) == hb_map_t {pair (1u, 2u)}); + assert (m2.get (hb_map_t ()) == hb_map_t {pair (1u, 2u)}); assert (m1.get (hb_map_t {pair (1u, 2u)}) == hb_map_t {pair (2u, 3u)}); assert (m2.get (hb_map_t {pair (1u, 2u)}) == hb_map_t {pair (2u, 3u)}); @@ -190,9 +189,8 @@ main (int argc, char **argv) m1.set (hb_set_t {1, 1000}, hb_set_t {2}); m2.set (hb_set_t {1, 1000}, hb_set_t {2}); - /* Cannot override empty set. */ - assert (m1.get (hb_set_t ()) == hb_set_t ()); - assert (m2.get (hb_set_t ()) == hb_set_t ()); + assert (m1.get (hb_set_t ()) == hb_set_t {1}); + assert (m2.get (hb_set_t ()) == hb_set_t {1}); assert (m1.get (hb_set_t {1000, 1}) == hb_set_t {2}); assert (m2.get (hb_set_t {1000, 1}) == hb_set_t {2}); @@ -214,9 +212,8 @@ main (int argc, char **argv) m1.set (vector_t {1}, vector_t {2}); m2.set (vector_t {1}, vector_t {2}); - /* Cannot override empty vector. */ - assert (m1.get (vector_t ()) == vector_t ()); - assert (m2.get (vector_t ()) == vector_t ()); + assert (m1.get (vector_t ()) == vector_t {1}); + assert (m2.get (vector_t ()) == vector_t {1}); assert (m1.get (vector_t {1}) == vector_t {2}); assert (m2.get (vector_t {1}) == vector_t {2}); From 0ccab339f98ab7af27b3ca0db8489ff836ca11f3 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Thu, 2 Jun 2022 10:43:36 -0600 Subject: [PATCH 04/23] [map] Remove invalid-key template arguments since unused --- src/hb-map.hh | 6 ------ src/hb-ot-post-table-v2subset.hh | 2 +- src/hb-serialize.hh | 3 +-- src/test-map.cc | 26 +++++++++++++------------- 4 files changed, 15 insertions(+), 22 deletions(-) diff --git a/src/hb-map.hh b/src/hb-map.hh index a0ab17a23..c62b9d7bb 100644 --- a/src/hb-map.hh +++ b/src/hb-map.hh @@ -35,9 +35,7 @@ */ template ::value ? 0 : std::is_signed::value ? hb_int_min (K) : (K) -1, v_invalid_t vINVALID = std::is_pointer::value ? 0 : std::is_signed::value ? hb_int_min (V) : (V) -1> struct hb_hashmap_t { @@ -401,15 +399,11 @@ struct hb_hashmap_t struct hb_map_t : hb_hashmap_t { using hashmap = hb_hashmap_t; ~hb_map_t () = default; diff --git a/src/hb-ot-post-table-v2subset.hh b/src/hb-ot-post-table-v2subset.hh index c817b28e6..8c0312fa4 100644 --- a/src/hb-ot-post-table-v2subset.hh +++ b/src/hb-ot-post-table-v2subset.hh @@ -78,7 +78,7 @@ HB_INTERNAL bool postV2Tail::subset (hb_subset_context_t *c) const post::accelerator_t _post (c->plan->source); - hb_hashmap_t glyph_name_to_new_index; + hb_hashmap_t glyph_name_to_new_index; for (hb_codepoint_t new_gid = 0; new_gid < num_glyphs; new_gid++) { hb_codepoint_t old_gid = reverse_glyph_map.get (new_gid); diff --git a/src/hb-serialize.hh b/src/hb-serialize.hh index 5663b290c..3f4535ce4 100644 --- a/src/hb-serialize.hh +++ b/src/hb-serialize.hh @@ -720,8 +720,7 @@ struct hb_serialize_context_t /* Map view of packed objects. */ hb_hashmap_t packed_map; + objidx_t, 0> packed_map; }; #endif /* HB_SERIALIZE_HH */ diff --git a/src/test-map.cc b/src/test-map.cc index a5e85074c..e62f65091 100644 --- a/src/test-map.cc +++ b/src/test-map.cc @@ -127,19 +127,19 @@ main (int argc, char **argv) /* Test class key / value types. */ { - hb_hashmap_t m1; - hb_hashmap_t m2; - hb_hashmap_t m3; + hb_hashmap_t m1; + hb_hashmap_t m2; + hb_hashmap_t m3; assert (m1.get_population () == 0); assert (m2.get_population () == 0); assert (m3.get_population () == 0); } { - hb_hashmap_t m0; - hb_hashmap_t m1; - hb_hashmap_t m2; - hb_hashmap_t m3; + hb_hashmap_t m0; + hb_hashmap_t m1; + hb_hashmap_t m2; + hb_hashmap_t m3; std::string s; for (unsigned i = 1; i < 1000; i++) @@ -156,8 +156,8 @@ main (int argc, char **argv) { using pair = hb_pair_t; - hb_hashmap_t m1; - hb_hashmap_t m2; + hb_hashmap_t m1; + hb_hashmap_t m2; m1.set (hb_map_t (), hb_map_t {}); m2.set (hb_map_t (), hb_map_t {}); @@ -177,8 +177,8 @@ main (int argc, char **argv) /* Test hashing sets. */ { - hb_hashmap_t m1; - hb_hashmap_t m2; + hb_hashmap_t m1; + hb_hashmap_t m2; m1.set (hb_set_t (), hb_set_t ()); m2.set (hb_set_t (), hb_set_t ()); @@ -200,8 +200,8 @@ main (int argc, char **argv) { using vector_t = hb_vector_t; - hb_hashmap_t m1; - hb_hashmap_t m2; + hb_hashmap_t m1; + hb_hashmap_t m2; m1.set (vector_t (), vector_t ()); m2.set (vector_t (), vector_t ()); From 3f78a71ca059b461f79d0ee64766c7c3f4327b14 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Thu, 2 Jun 2022 11:11:35 -0600 Subject: [PATCH 05/23] [map] Finally! Just can usd hb_hashmap_t Yay! --- src/hb-map.hh | 29 ++++++++++----------- src/hb-ot-post-table-v2subset.hh | 2 +- src/hb-serialize.hh | 3 +-- src/test-map.cc | 43 ++++++++------------------------ 4 files changed, 27 insertions(+), 50 deletions(-) diff --git a/src/hb-map.hh b/src/hb-map.hh index c62b9d7bb..9b435c7d7 100644 --- a/src/hb-map.hh +++ b/src/hb-map.hh @@ -35,8 +35,7 @@ */ template ::value ? 0 : std::is_signed::value ? hb_int_min (V) : (V) -1> + bool minus_one = false> struct hb_hashmap_t { hb_hashmap_t () { init (); } @@ -71,19 +70,24 @@ struct hb_hashmap_t uint32_t is_tombstone_ : 1; V value; - - bool is_used () const { return is_used_; } void set_used (bool is_used) { is_used_ = is_used; } bool is_tombstone () const { return is_tombstone_; } void set_tombstone (bool is_tombstone) { is_tombstone_ = is_tombstone; } bool is_real () const { return is_used_ && !is_tombstone_; } + template + static V default_value () { return V(); }; + template + static V default_value () { return V(-1); }; + void clear () { new (std::addressof (key)) K (); new (std::addressof (value)) V (); - value = hb_coerce (vINVALID); + value = default_value (); hash = 0; is_used_ = false; is_tombstone_ = false; @@ -200,12 +204,12 @@ struct hb_hashmap_t V get (K key) const { - if (unlikely (!items)) return hb_coerce (vINVALID); + if (unlikely (!items)) return item_t::default_value (); unsigned int i = bucket_for (key); - return items[i].is_real () && items[i] == key ? items[i].value : hb_coerce (vINVALID); + return items[i].is_real () && items[i] == key ? items[i].value : item_t::default_value (); } - void del (K key) { set_with_hash (key, hb_hash (key), hb_coerce (vINVALID), true); } + void del (K key) { set_with_hash (key, hb_hash (key), item_t::default_value (), true); } /* Has interface. */ typedef V value_t; @@ -214,8 +218,7 @@ struct hb_hashmap_t { const V &v = (*this)[k]; if (vp) *vp = v; - const V vinv = hb_coerce (vINVALID); - return v != vinv; + return v != item_t::default_value (); // TODO } /* Projection. */ V operator () (K k) const { return get (k); } @@ -398,13 +401,11 @@ struct hb_hashmap_t struct hb_map_t : hb_hashmap_t + true> { using hashmap = hb_hashmap_t; + true>; ~hb_map_t () = default; hb_map_t () : hashmap () {} diff --git a/src/hb-ot-post-table-v2subset.hh b/src/hb-ot-post-table-v2subset.hh index 8c0312fa4..718b09d7e 100644 --- a/src/hb-ot-post-table-v2subset.hh +++ b/src/hb-ot-post-table-v2subset.hh @@ -78,7 +78,7 @@ HB_INTERNAL bool postV2Tail::subset (hb_subset_context_t *c) const post::accelerator_t _post (c->plan->source); - hb_hashmap_t glyph_name_to_new_index; + hb_hashmap_t glyph_name_to_new_index; for (hb_codepoint_t new_gid = 0; new_gid < num_glyphs; new_gid++) { hb_codepoint_t old_gid = reverse_glyph_map.get (new_gid); diff --git a/src/hb-serialize.hh b/src/hb-serialize.hh index 3f4535ce4..6f56faafe 100644 --- a/src/hb-serialize.hh +++ b/src/hb-serialize.hh @@ -719,8 +719,7 @@ struct hb_serialize_context_t hb_vector_t packed; /* Map view of packed objects. */ - hb_hashmap_t packed_map; + hb_hashmap_t packed_map; }; #endif /* HB_SERIALIZE_HH */ diff --git a/src/test-map.cc b/src/test-map.cc index e62f65091..0351d8161 100644 --- a/src/test-map.cc +++ b/src/test-map.cc @@ -27,11 +27,6 @@ #include "hb-set.hh" #include -static const std::string invalid{"invalid"}; -static const hb_map_t invalid_map{}; -static const hb_set_t invalid_set{}; -static const hb_vector_t invalid_vector{}; - int main (int argc, char **argv) { @@ -127,19 +122,19 @@ main (int argc, char **argv) /* Test class key / value types. */ { - hb_hashmap_t m1; - hb_hashmap_t m2; - hb_hashmap_t m3; + hb_hashmap_t m1; + hb_hashmap_t m2; + hb_hashmap_t m3; assert (m1.get_population () == 0); assert (m2.get_population () == 0); assert (m3.get_population () == 0); } { - hb_hashmap_t m0; - hb_hashmap_t m1; - hb_hashmap_t m2; - hb_hashmap_t m3; + hb_hashmap_t m0; + hb_hashmap_t m1; + hb_hashmap_t m2; + hb_hashmap_t m3; std::string s; for (unsigned i = 1; i < 1000; i++) @@ -156,67 +151,49 @@ main (int argc, char **argv) { using pair = hb_pair_t; - hb_hashmap_t m1; - hb_hashmap_t m2; + hb_hashmap_t m1; m1.set (hb_map_t (), hb_map_t {}); - m2.set (hb_map_t (), hb_map_t {}); m1.set (hb_map_t (), hb_map_t {pair (1u, 2u)}); - m2.set (hb_map_t (), hb_map_t {pair (1u, 2u)}); m1.set (hb_map_t {pair (1u, 2u)}, hb_map_t {pair (2u, 3u)}); - m2.set (hb_map_t {pair (1u, 2u)}, hb_map_t {pair (2u, 3u)}); assert (m1.get (hb_map_t ()) == hb_map_t {pair (1u, 2u)}); - assert (m2.get (hb_map_t ()) == hb_map_t {pair (1u, 2u)}); assert (m1.get (hb_map_t {pair (1u, 2u)}) == hb_map_t {pair (2u, 3u)}); - assert (m2.get (hb_map_t {pair (1u, 2u)}) == hb_map_t {pair (2u, 3u)}); } /* Test hashing sets. */ { - hb_hashmap_t m1; - hb_hashmap_t m2; + hb_hashmap_t m1; m1.set (hb_set_t (), hb_set_t ()); - m2.set (hb_set_t (), hb_set_t ()); m1.set (hb_set_t (), hb_set_t {1}); - m2.set (hb_set_t (), hb_set_t {1}); m1.set (hb_set_t {1, 1000}, hb_set_t {2}); - m2.set (hb_set_t {1, 1000}, hb_set_t {2}); assert (m1.get (hb_set_t ()) == hb_set_t {1}); - assert (m2.get (hb_set_t ()) == hb_set_t {1}); assert (m1.get (hb_set_t {1000, 1}) == hb_set_t {2}); - assert (m2.get (hb_set_t {1000, 1}) == hb_set_t {2}); } /* Test hashing vectors. */ { using vector_t = hb_vector_t; - hb_hashmap_t m1; - hb_hashmap_t m2; + hb_hashmap_t m1; m1.set (vector_t (), vector_t ()); - m2.set (vector_t (), vector_t ()); m1.set (vector_t (), vector_t {1}); - m2.set (vector_t (), vector_t {1}); m1.set (vector_t {1}, vector_t {2}); - m2.set (vector_t {1}, vector_t {2}); assert (m1.get (vector_t ()) == vector_t {1}); - assert (m2.get (vector_t ()) == vector_t {1}); assert (m1.get (vector_t {1}) == vector_t {2}); - assert (m2.get (vector_t {1}) == vector_t {2}); } /* Test hb::shared_ptr. */ From 97ea10a63a0be388bfb02ae203c468533304bda0 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Thu, 2 Jun 2022 11:14:17 -0600 Subject: [PATCH 06/23] Remove old nullptr_t hacks Were used for hashmap before. --- src/hb-array.hh | 2 -- src/hb-map.hh | 2 -- src/hb-set.hh | 1 - 3 files changed, 5 deletions(-) diff --git a/src/hb-array.hh b/src/hb-array.hh index 1963698cf..5baeb6f7f 100644 --- a/src/hb-array.hh +++ b/src/hb-array.hh @@ -56,7 +56,6 @@ struct hb_array_t : hb_iter_with_fallback_t, Type&> hb_array_t& operator= (const hb_array_t&) = default; hb_array_t& operator= (hb_array_t&&) = default; - constexpr hb_array_t (std::nullptr_t) : hb_array_t () {} constexpr hb_array_t (Type *array_, unsigned int length_) : arrayZ (array_), length (length_) {} template constexpr hb_array_t (Type (&array_)[length_]) : hb_array_t (array_, length_) {} @@ -314,7 +313,6 @@ struct hb_sorted_array_t : hb_sorted_array_t& operator= (const hb_sorted_array_t&) = default; hb_sorted_array_t& operator= (hb_sorted_array_t&&) = default; - constexpr hb_sorted_array_t (std::nullptr_t) : hb_sorted_array_t () {} constexpr hb_sorted_array_t (Type *array_, unsigned int length_) : hb_array_t (array_, length_) {} template constexpr hb_sorted_array_t (Type (&array_)[length_]) : hb_array_t (array_) {} diff --git a/src/hb-map.hh b/src/hb-map.hh index 9b435c7d7..fb3d2d1c7 100644 --- a/src/hb-map.hh +++ b/src/hb-map.hh @@ -39,7 +39,6 @@ template ~hb_set_t () = default; hb_set_t () : sparseset () {}; - hb_set_t (std::nullptr_t) : hb_set_t () {}; hb_set_t (const hb_set_t &o) : sparseset ((sparseset &) o) {}; hb_set_t (hb_set_t&& o) : sparseset (std::move ((sparseset &) o)) {} hb_set_t& operator = (const hb_set_t&) = default; From b9230c542558afac93f1fb6d7ca1442a06688d38 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Thu, 2 Jun 2022 11:18:38 -0600 Subject: [PATCH 07/23] [map] Fix has() --- src/hb-map.hh | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/src/hb-map.hh b/src/hb-map.hh index fb3d2d1c7..065ae1731 100644 --- a/src/hb-map.hh +++ b/src/hb-map.hh @@ -213,11 +213,24 @@ struct hb_hashmap_t /* Has interface. */ typedef V value_t; value_t operator [] (K k) const { return get (k); } - bool has (K k, V *vp = nullptr) const + bool has (K key, V *vp = nullptr) const { - const V &v = (*this)[k]; - if (vp) *vp = v; - return v != item_t::default_value (); // TODO + if (unlikely (!items)) + { + if (vp) *vp = item_t::default_value (); + return false; + } + unsigned int i = bucket_for (key); + if (items[i].is_real () && items[i] == key) + { + if (vp) *vp = items[i].value; + return true; + } + else + { + if (vp) *vp = item_t::default_value (); + return false; + } } /* Projection. */ V operator () (K k) const { return get (k); } From 4c1b5d9ece8b59eb5346a8eaeaad09dfeb8dfd7f Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Thu, 2 Jun 2022 11:25:11 -0600 Subject: [PATCH 08/23] Whitespace --- src/test-map.cc | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/test-map.cc b/src/test-map.cc index 0351d8161..65f11243f 100644 --- a/src/test-map.cc +++ b/src/test-map.cc @@ -154,13 +154,10 @@ main (int argc, char **argv) hb_hashmap_t m1; m1.set (hb_map_t (), hb_map_t {}); - m1.set (hb_map_t (), hb_map_t {pair (1u, 2u)}); - m1.set (hb_map_t {pair (1u, 2u)}, hb_map_t {pair (2u, 3u)}); assert (m1.get (hb_map_t ()) == hb_map_t {pair (1u, 2u)}); - assert (m1.get (hb_map_t {pair (1u, 2u)}) == hb_map_t {pair (2u, 3u)}); } @@ -169,13 +166,10 @@ main (int argc, char **argv) hb_hashmap_t m1; m1.set (hb_set_t (), hb_set_t ()); - m1.set (hb_set_t (), hb_set_t {1}); - m1.set (hb_set_t {1, 1000}, hb_set_t {2}); assert (m1.get (hb_set_t ()) == hb_set_t {1}); - assert (m1.get (hb_set_t {1000, 1}) == hb_set_t {2}); } @@ -186,13 +180,10 @@ main (int argc, char **argv) hb_hashmap_t m1; m1.set (vector_t (), vector_t ()); - m1.set (vector_t (), vector_t {1}); - m1.set (vector_t {1}, vector_t {2}); assert (m1.get (vector_t ()) == vector_t {1}); - assert (m1.get (vector_t {1}) == vector_t {2}); } From f0a0dcad703ca1db037687b4c59ce11668f38ca6 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Thu, 2 Jun 2022 11:25:56 -0600 Subject: [PATCH 09/23] [test-map] Test hashing shared_ptr --- src/test-map.cc | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/test-map.cc b/src/test-map.cc index 65f11243f..c4fdf45c1 100644 --- a/src/test-map.cc +++ b/src/test-map.cc @@ -189,14 +189,12 @@ main (int argc, char **argv) /* Test hb::shared_ptr. */ hb_hash (hb::shared_ptr ()); -#if 0 { - hb_hashmap_t, hb::shared_ptr, std::nullptr_t, std::nullptr_t, nullptr, nullptr> m; + hb_hashmap_t, hb::shared_ptr> m; m.get (hb::shared_ptr ()); m.get (hb::shared_ptr (hb_set_get_empty ())); } -#endif return 0; } From a42a703cb62c84b2ed141e64570e1f1a2d74695e Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Thu, 2 Jun 2022 11:51:20 -0600 Subject: [PATCH 10/23] [shared_ptr] Clear p in destructor --- src/hb-cplusplus.hh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hb-cplusplus.hh b/src/hb-cplusplus.hh index afad2d1b8..76122523e 100644 --- a/src/hb-cplusplus.hh +++ b/src/hb-cplusplus.hh @@ -59,7 +59,7 @@ struct shared_ptr shared_ptr (shared_ptr &&o) : p (o.p) { o.p = nullptr; } shared_ptr& operator = (const shared_ptr &o) { if (p != o.p) { destroy (); p = o.p; reference (); } return *this; } shared_ptr& operator = (shared_ptr &&o) { destroy (); p = o.p; o.p = nullptr; return *this; } - ~shared_ptr () { v::destroy (p); } + ~shared_ptr () { v::destroy (p); p = nullptr; } T* get() const { return p; } From e9407a2bd25f80d65559b6a869585033e2a08b24 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Thu, 2 Jun 2022 11:29:44 -0600 Subject: [PATCH 11/23] Use shared_ptr in one place See if valgrind is happy... --- src/hb-map.hh | 2 +- src/hb-ot-layout-gsubgpos.hh | 9 +++------ src/hb-ot-layout.cc | 10 ++-------- 3 files changed, 6 insertions(+), 15 deletions(-) diff --git a/src/hb-map.hh b/src/hb-map.hh index 065ae1731..d3bb49a87 100644 --- a/src/hb-map.hh +++ b/src/hb-map.hh @@ -198,7 +198,7 @@ struct hb_hashmap_t return true; } - bool set (K key, const V& value) { return set_with_hash (key, hb_hash (key), value); } + bool set (K key, const V &value) { return set_with_hash (key, hb_hash (key), value); } bool set (K key, V&& value) { return set_with_hash (key, hb_hash (key), std::move (value)); } V get (K key) const diff --git a/src/hb-ot-layout-gsubgpos.hh b/src/hb-ot-layout-gsubgpos.hh index fff5974d8..048278500 100644 --- a/src/hb-ot-layout-gsubgpos.hh +++ b/src/hb-ot-layout-gsubgpos.hh @@ -111,12 +111,9 @@ struct hb_closure_context_t : if (!done_lookups_glyph_set->get (lookup_index)) { - hb_set_t* empty_set = hb_set_create (); + hb::shared_ptr empty_set {hb_set_create ()}; if (unlikely (!done_lookups_glyph_set->set (lookup_index, empty_set))) - { - hb_set_destroy (empty_set); return true; - } } hb_set_clear (done_lookups_glyph_set->get (lookup_index)); @@ -171,7 +168,7 @@ struct hb_closure_context_t : hb_closure_context_t (hb_face_t *face_, hb_set_t *glyphs_, hb_map_t *done_lookups_glyph_count_, - hb_hashmap_t *done_lookups_glyph_set_, + hb_hashmap_t> *done_lookups_glyph_set_, unsigned int nesting_level_left_ = HB_MAX_NESTING_LEVEL) : face (face_), glyphs (glyphs_), @@ -195,7 +192,7 @@ struct hb_closure_context_t : private: hb_map_t *done_lookups_glyph_count; - hb_hashmap_t *done_lookups_glyph_set; + hb_hashmap_t> *done_lookups_glyph_set; unsigned int lookup_count = 0; }; diff --git a/src/hb-ot-layout.cc b/src/hb-ot-layout.cc index 9a8070a4a..f5031be81 100644 --- a/src/hb-ot-layout.cc +++ b/src/hb-ot-layout.cc @@ -1501,15 +1501,12 @@ hb_ot_layout_lookup_substitute_closure (hb_face_t *face, hb_set_t *glyphs /* OUT */) { hb_map_t done_lookups_glyph_count; - hb_hashmap_t done_lookups_glyph_set; + hb_hashmap_t> done_lookups_glyph_set; OT::hb_closure_context_t c (face, glyphs, &done_lookups_glyph_count, &done_lookups_glyph_set); const OT::SubstLookup& l = face->table.GSUB->table->get_lookup (lookup_index); l.closure (&c, lookup_index); - - for (auto _ : done_lookups_glyph_set.iter ()) - hb_set_destroy (_.second); } /** @@ -1529,7 +1526,7 @@ hb_ot_layout_lookups_substitute_closure (hb_face_t *face, hb_set_t *glyphs /* OUT */) { hb_map_t done_lookups_glyph_count; - hb_hashmap_t done_lookups_glyph_set; + hb_hashmap_t> done_lookups_glyph_set; OT::hb_closure_context_t c (face, glyphs, &done_lookups_glyph_count, &done_lookups_glyph_set); const GSUB& gsub = *face->table.GSUB->table; @@ -1551,9 +1548,6 @@ hb_ot_layout_lookups_substitute_closure (hb_face_t *face, } } while (iteration_count++ <= HB_CLOSURE_MAX_STAGES && glyphs_length != glyphs->get_population ()); - - for (auto _ : done_lookups_glyph_set.iter ()) - hb_set_destroy (_.second); } /* From bca710e8ad2cccaa013e19a63c58899e45284df8 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Thu, 2 Jun 2022 12:06:25 -0600 Subject: [PATCH 12/23] [gsubgpos] Use map has() instead of get() when appropriate --- src/hb-ot-layout-gsubgpos.hh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hb-ot-layout-gsubgpos.hh b/src/hb-ot-layout-gsubgpos.hh index 048278500..276a3601c 100644 --- a/src/hb-ot-layout-gsubgpos.hh +++ b/src/hb-ot-layout-gsubgpos.hh @@ -109,7 +109,7 @@ struct hb_closure_context_t : { done_lookups_glyph_count->set (lookup_index, glyphs->get_population ()); - if (!done_lookups_glyph_set->get (lookup_index)) + if (!done_lookups_glyph_set->has (lookup_index)) { hb::shared_ptr empty_set {hb_set_create ()}; if (unlikely (!done_lookups_glyph_set->set (lookup_index, empty_set))) From d7785a6da0a5ced69203270a48a9a4da9e8f230a Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Thu, 2 Jun 2022 12:42:24 -0600 Subject: [PATCH 13/23] [cplusplus] Add unique_ptr --- src/hb-algs.hh | 5 +++++ src/hb-cplusplus.hh | 44 +++++++++++++++++++++++++++++++++++++- test/api/test-cplusplus.cc | 2 ++ 3 files changed, 50 insertions(+), 1 deletion(-) diff --git a/src/hb-algs.hh b/src/hb-algs.hh index 34e512e7e..ce4849b01 100644 --- a/src/hb-algs.hh +++ b/src/hb-algs.hh @@ -247,6 +247,11 @@ struct { return v.get () ? v.get ()->hash () : 0; } + template constexpr uint32_t + impl (const hb::unique_ptr& v, hb_priority<1>) const + { + return v.get () ? v.get ()->hash () : 0; + } template constexpr auto impl (const T& v, hb_priority<0>) const HB_RETURN (uint32_t, std::hash>{} (hb_deref (v))) diff --git a/src/hb-cplusplus.hh b/src/hb-cplusplus.hh index 76122523e..86d045208 100644 --- a/src/hb-cplusplus.hh +++ b/src/hb-cplusplus.hh @@ -58,7 +58,7 @@ struct shared_ptr shared_ptr (const shared_ptr &o) : p (v::reference (o.p)) {} shared_ptr (shared_ptr &&o) : p (o.p) { o.p = nullptr; } shared_ptr& operator = (const shared_ptr &o) { if (p != o.p) { destroy (); p = o.p; reference (); } return *this; } - shared_ptr& operator = (shared_ptr &&o) { destroy (); p = o.p; o.p = nullptr; return *this; } + shared_ptr& operator = (shared_ptr &&o) { v::destroy (p); p = o.p; o.p = nullptr; return *this; } ~shared_ptr () { v::destroy (p); p = nullptr; } T* get() const { return p; } @@ -89,6 +89,38 @@ struct shared_ptr template struct is_shared_ptr : std::false_type {}; template struct is_shared_ptr> : std::true_type {}; +template +struct unique_ptr +{ + using element_type = T; + + using v = vtable; + + explicit unique_ptr (T *p = nullptr) : p (p) {} + unique_ptr (const unique_ptr &o) = delete; + unique_ptr (unique_ptr &&o) : p (o.p) { o.p = nullptr; } + unique_ptr& operator = (const unique_ptr &o) = delete; + unique_ptr& operator = (unique_ptr &&o) { v::destroy (p); p = o.p; o.p = nullptr; return *this; } + ~unique_ptr () { v::destroy (p); p = nullptr; } + + T* get() const { return p; } + T* release () { T* v = p; p = nullptr; return v; } + + void swap (unique_ptr &o) { std::swap (p, o.p); } + friend void swap (unique_ptr &a, unique_ptr &b) { std::swap (a.p, b.p); } + + operator T * () const { return p; } + T& operator * () const { return *get (); } + T* operator -> () const { return get (); } + operator bool () { return p; } + + private: + T *p; +}; + +template struct is_unique_ptr : std::false_type {}; +template struct is_unique_ptr> : std::true_type {}; + template > } }; +template +struct std::hash> +{ + std::size_t operator()(const hb::unique_ptr& v) const noexcept + { + std::size_t h = std::hash{}(v.get ()); + return h; + } +}; + #endif /* __cplusplus */ diff --git a/test/api/test-cplusplus.cc b/test/api/test-cplusplus.cc index d23e794c4..d742750f6 100644 --- a/test/api/test-cplusplus.cc +++ b/test/api/test-cplusplus.cc @@ -106,5 +106,7 @@ main () pb.set_user_data (&key, b, nullptr, true); (void) pb.get_user_data (&key); + hb::unique_ptr pb5 {pb3.reference ()}; + return pb == pb.get_empty () || pb == pb2; } From 8bb2a3326e92d553b9bea7462574a2d44782cbfd Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Thu, 2 Jun 2022 15:18:23 -0600 Subject: [PATCH 14/23] [map] Remove unneeded assignment --- src/hb-map.hh | 1 - 1 file changed, 1 deletion(-) diff --git a/src/hb-map.hh b/src/hb-map.hh index d3bb49a87..262954c34 100644 --- a/src/hb-map.hh +++ b/src/hb-map.hh @@ -86,7 +86,6 @@ struct hb_hashmap_t { new (std::addressof (key)) K (); new (std::addressof (value)) V (); - value = default_value (); hash = 0; is_used_ = false; is_tombstone_ = false; From 997d9cc466abfb9031f46d1baef5a2cb3164f7cc Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Thu, 2 Jun 2022 18:04:12 -0600 Subject: [PATCH 15/23] [map] Make unique_ptr hashable --- src/hb-bimap.hh | 3 ++- src/hb-map.hh | 24 ++++++++++++------------ src/hb-ot-color-cpal-table.hh | 4 ++-- src/hb-ot-layout-common.hh | 4 ++-- src/hb-ot-layout-gsubgpos.hh | 7 +++---- src/hb-ot-layout.cc | 4 ++-- src/hb-ot-post-table-v2subset.hh | 4 +++- src/hb-repacker.hh | 16 ++++++++-------- src/test-map.cc | 8 ++++++++ 9 files changed, 42 insertions(+), 32 deletions(-) diff --git a/src/hb-bimap.hh b/src/hb-bimap.hh index 8ce4a5e80..8e8c98871 100644 --- a/src/hb-bimap.hh +++ b/src/hb-bimap.hh @@ -64,7 +64,8 @@ struct hb_bimap_t hb_codepoint_t backward (hb_codepoint_t rhs) const { return back_map.get (rhs); } hb_codepoint_t operator [] (hb_codepoint_t lhs) const { return get (lhs); } - bool has (hb_codepoint_t lhs, hb_codepoint_t *vp = nullptr) const { return forw_map.has (lhs, vp); } + bool has (hb_codepoint_t lhs) const { return forw_map.has (lhs); } + void del (hb_codepoint_t lhs) { diff --git a/src/hb-map.hh b/src/hb-map.hh index 262954c34..028e4654b 100644 --- a/src/hb-map.hh +++ b/src/hb-map.hh @@ -77,10 +77,10 @@ struct hb_hashmap_t template - static V default_value () { return V(); }; + static const V& default_value () { return Null(V); }; template - static V default_value () { return V(-1); }; + static const V& default_value () { static const V minus_1 = -1; return minus_1; }; void clear () { @@ -197,10 +197,10 @@ struct hb_hashmap_t return true; } - bool set (K key, const V &value) { return set_with_hash (key, hb_hash (key), value); } - bool set (K key, V&& value) { return set_with_hash (key, hb_hash (key), std::move (value)); } + template + bool set (K key, VV&& value) { return set_with_hash (key, hb_hash (key), std::forward (value)); } - V get (K key) const + const V& get (K key) const { if (unlikely (!items)) return item_t::default_value (); unsigned int i = bucket_for (key); @@ -212,22 +212,22 @@ struct hb_hashmap_t /* Has interface. */ typedef V value_t; value_t operator [] (K k) const { return get (k); } - bool has (K key, V *vp = nullptr) const + bool has (K key, const V **vp = nullptr) const { if (unlikely (!items)) { - if (vp) *vp = item_t::default_value (); + if (vp) *vp = &item_t::default_value (); return false; } unsigned int i = bucket_for (key); if (items[i].is_real () && items[i] == key) { - if (vp) *vp = items[i].value; + if (vp) *vp = &items[i].value; return true; } else { - if (vp) *vp = item_t::default_value (); + if (vp) *vp = &item_t::default_value (); return false; } } @@ -320,7 +320,7 @@ struct hb_hashmap_t } items[i].key = key; - items[i].value = value; + items[i].value = std::forward (value); items[i].hash = hash; items[i].set_used (true); items[i].set_tombstone (is_delete); @@ -332,12 +332,12 @@ struct hb_hashmap_t return true; } - unsigned int bucket_for (K key) const + unsigned int bucket_for (const K &key) const { return bucket_for_hash (key, hb_hash (key)); } - unsigned int bucket_for_hash (K key, uint32_t hash) const + unsigned int bucket_for_hash (const K &key, uint32_t hash) const { hash &= 0x3FFFFFFF; // We only store lower 30bit of hash unsigned int i = hash % prime; diff --git a/src/hb-ot-color-cpal-table.hh b/src/hb-ot-color-cpal-table.hh index 201fa3e46..bcab77f79 100644 --- a/src/hb-ot-color-cpal-table.hh +++ b/src/hb-ot-color-cpal-table.hh @@ -97,10 +97,10 @@ struct CPALV1Tail c->push (); for (const auto _ : colorLabels) { - hb_codepoint_t v; + const hb_codepoint_t *v; if (!color_index_map->has (_, &v)) continue; NameID new_color_idx; - new_color_idx = v; + new_color_idx = *v; if (!c->copy (new_color_idx)) { c->pop_discard (); diff --git a/src/hb-ot-layout-common.hh b/src/hb-ot-layout-common.hh index f9a11657d..a195363d7 100644 --- a/src/hb-ot-layout-common.hh +++ b/src/hb-ot-layout-common.hh @@ -659,8 +659,8 @@ struct LangSys auto *out = c->serializer->start_embed (*this); if (unlikely (!out || !c->serializer->extend_min (out))) return_trace (false); - unsigned v; - out->reqFeatureIndex = l->feature_index_map->has (reqFeatureIndex, &v) ? v : 0xFFFFu; + const unsigned *v; + out->reqFeatureIndex = l->feature_index_map->has (reqFeatureIndex, &v) ? *v : 0xFFFFu; if (!l->visitFeatureIndex (featureIndex.len)) return_trace (false); diff --git a/src/hb-ot-layout-gsubgpos.hh b/src/hb-ot-layout-gsubgpos.hh index 276a3601c..b76b43139 100644 --- a/src/hb-ot-layout-gsubgpos.hh +++ b/src/hb-ot-layout-gsubgpos.hh @@ -111,8 +111,7 @@ struct hb_closure_context_t : if (!done_lookups_glyph_set->has (lookup_index)) { - hb::shared_ptr empty_set {hb_set_create ()}; - if (unlikely (!done_lookups_glyph_set->set (lookup_index, empty_set))) + if (unlikely (!done_lookups_glyph_set->set (lookup_index, hb::unique_ptr {hb_set_create ()}))) return true; } @@ -168,7 +167,7 @@ struct hb_closure_context_t : hb_closure_context_t (hb_face_t *face_, hb_set_t *glyphs_, hb_map_t *done_lookups_glyph_count_, - hb_hashmap_t> *done_lookups_glyph_set_, + hb_hashmap_t> *done_lookups_glyph_set_, unsigned int nesting_level_left_ = HB_MAX_NESTING_LEVEL) : face (face_), glyphs (glyphs_), @@ -192,7 +191,7 @@ struct hb_closure_context_t : private: hb_map_t *done_lookups_glyph_count; - hb_hashmap_t> *done_lookups_glyph_set; + hb_hashmap_t> *done_lookups_glyph_set; unsigned int lookup_count = 0; }; diff --git a/src/hb-ot-layout.cc b/src/hb-ot-layout.cc index f5031be81..35e887ba3 100644 --- a/src/hb-ot-layout.cc +++ b/src/hb-ot-layout.cc @@ -1501,7 +1501,7 @@ hb_ot_layout_lookup_substitute_closure (hb_face_t *face, hb_set_t *glyphs /* OUT */) { hb_map_t done_lookups_glyph_count; - hb_hashmap_t> done_lookups_glyph_set; + hb_hashmap_t> done_lookups_glyph_set; OT::hb_closure_context_t c (face, glyphs, &done_lookups_glyph_count, &done_lookups_glyph_set); const OT::SubstLookup& l = face->table.GSUB->table->get_lookup (lookup_index); @@ -1526,7 +1526,7 @@ hb_ot_layout_lookups_substitute_closure (hb_face_t *face, hb_set_t *glyphs /* OUT */) { hb_map_t done_lookups_glyph_count; - hb_hashmap_t> done_lookups_glyph_set; + hb_hashmap_t> done_lookups_glyph_set; OT::hb_closure_context_t c (face, glyphs, &done_lookups_glyph_count, &done_lookups_glyph_set); const GSUB& gsub = *face->table.GSUB->table; diff --git a/src/hb-ot-post-table-v2subset.hh b/src/hb-ot-post-table-v2subset.hh index 718b09d7e..c8a4429eb 100644 --- a/src/hb-ot-post-table-v2subset.hh +++ b/src/hb-ot-post-table-v2subset.hh @@ -85,9 +85,11 @@ HB_INTERNAL bool postV2Tail::subset (hb_subset_context_t *c) const unsigned old_index = glyphNameIndex[old_gid]; unsigned new_index; + const unsigned *new_index2; if (old_index <= 257) new_index = old_index; - else if (!old_new_index_map.has (old_index, &new_index)) + else if (!old_new_index_map.has (old_index, &new_index2)) { + new_index = *new_index2; hb_bytes_t s = _post.find_glyph_name (old_gid); new_index = glyph_name_to_new_index.get (s); if (new_index == (unsigned)-1) diff --git a/src/hb-repacker.hh b/src/hb-repacker.hh index 66dce8df8..c9075fa1e 100644 --- a/src/hb-repacker.hh +++ b/src/hb-repacker.hh @@ -430,8 +430,8 @@ struct graph_t auto new_subgraph = + subgraph.keys () | hb_map([&] (unsigned node_idx) { - unsigned v; - if (index_map.has (node_idx, &v)) return v; + const unsigned *v; + if (index_map.has (node_idx, &v)) return *v; return node_idx; }) ; @@ -443,11 +443,11 @@ struct graph_t unsigned next = HB_SET_VALUE_INVALID; while (roots.next (&next)) { - unsigned v; + const unsigned *v; if (index_map.has (next, &v)) { roots.del (next); - roots.add (v); + roots.add (*v); } } @@ -458,10 +458,10 @@ struct graph_t { for (const auto& link : vertices_[node_idx].obj.all_links ()) { - unsigned v; + const unsigned *v; if (subgraph.has (link.objidx, &v)) { - subgraph.set (link.objidx, v + 1); + subgraph.set (link.objidx, *v + 1); continue; } subgraph.set (link.objidx, 1); @@ -943,11 +943,11 @@ struct graph_t { for (auto& link : vertices_[i].obj.all_links_writer ()) { - unsigned v; + const unsigned *v; if (!id_map.has (link.objidx, &v)) continue; if (only_wide && !(link.width == 4 && !link.is_signed)) continue; - reassign_link (link, i, v); + reassign_link (link, i, *v); } } } diff --git a/src/test-map.cc b/src/test-map.cc index c4fdf45c1..9eef49e88 100644 --- a/src/test-map.cc +++ b/src/test-map.cc @@ -195,6 +195,14 @@ main (int argc, char **argv) m.get (hb::shared_ptr ()); m.get (hb::shared_ptr (hb_set_get_empty ())); } + /* Test hb::unique_ptr. */ + hb_hash (hb::unique_ptr ()); + { + hb_hashmap_t, hb::unique_ptr> m; + + m.get (hb::unique_ptr ()); + m.get (hb::unique_ptr (hb_set_get_empty ())); + } return 0; } From a7a688616ab348a66873df5577eec66a0f70206f Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Thu, 2 Jun 2022 18:59:15 -0600 Subject: [PATCH 16/23] [cmap] Convert another map use to unique_ptr --- src/hb-ot-cmap-table.hh | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/src/hb-ot-cmap-table.hh b/src/hb-ot-cmap-table.hh index 7e96d9c8b..9d95f518c 100644 --- a/src/hb-ot-cmap-table.hh +++ b/src/hb-ot-cmap-table.hh @@ -1476,26 +1476,17 @@ struct SubtableUnicodesCache { private: const void* base; - hb_hashmap_t cached_unicodes; + hb_hashmap_t> cached_unicodes; public: SubtableUnicodesCache(const void* cmap_base) : base(cmap_base), cached_unicodes() {} - ~SubtableUnicodesCache() - { - for (hb_set_t* s : cached_unicodes.values()) { - hb_set_destroy (s); - } - } hb_set_t* set_for(const EncodingRecord* record) { if (!cached_unicodes.has ((intptr_t) record)) { - hb_set_t* new_set = hb_set_create (); - if (!cached_unicodes.set ((intptr_t) record, new_set)) { - hb_set_destroy (new_set); + if (!cached_unicodes.set ((intptr_t) record, hb::unique_ptr {hb_set_create ()})) return hb_set_get_empty (); - } (base+record->subtable).collect_unicodes (cached_unicodes.get ((intptr_t) record)); } return cached_unicodes.get ((intptr_t) record); From 25f57230d58524d9165a9b33d1666f84005617d5 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Fri, 3 Jun 2022 01:11:22 -0600 Subject: [PATCH 17/23] [map] Return references from new iter_ref() --- src/hb-map.hh | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/hb-map.hh b/src/hb-map.hh index 028e4654b..f91da642d 100644 --- a/src/hb-map.hh +++ b/src/hb-map.hh @@ -94,6 +94,7 @@ struct hb_hashmap_t bool operator == (const K &o) { return hb_deref (key) == hb_deref (o); } bool operator == (const item_t &o) { return *this == o.key; } hb_pair_t get_pair() const { return hb_pair_t (key, value); } + hb_pair_t get_pair_ref() const { return hb_pair_t (key, value); } uint32_t total_hash () const { return (hash * 31) + hb_hash (value); } @@ -281,6 +282,12 @@ struct hb_hashmap_t | hb_filter (&item_t::is_real) | hb_map (&item_t::get_pair) ) + auto iter_ref () const HB_AUTO_RETURN + ( + + hb_array (items, mask ? mask + 1 : 0) + | hb_filter (&item_t::is_real) + | hb_map (&item_t::get_pair) + ) auto keys () const HB_AUTO_RETURN ( + hb_array (items, mask ? mask + 1 : 0) From f13a79548fca34663ec3f0f86de6f2e742a09ab9 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Fri, 3 Jun 2022 01:17:20 -0600 Subject: [PATCH 18/23] [subset] Convert another use of hashmap to unique_ptr --- src/hb-ot-layout-common.hh | 14 +++++--------- src/hb-ot-layout-gsubgpos.hh | 2 +- src/hb-subset-plan.cc | 8 +------- src/hb-subset-plan.hh | 4 ++-- 4 files changed, 9 insertions(+), 19 deletions(-) diff --git a/src/hb-ot-layout-common.hh b/src/hb-ot-layout-common.hh index a195363d7..015180778 100644 --- a/src/hb-ot-layout-common.hh +++ b/src/hb-ot-layout-common.hh @@ -102,7 +102,7 @@ static void ClassDef_remap_and_serialize ( struct hb_prune_langsys_context_t { hb_prune_langsys_context_t (const void *table_, - hb_hashmap_t *script_langsys_map_, + hb_hashmap_t> *script_langsys_map_, const hb_map_t *duplicate_feature_map_, hb_set_t *new_collected_feature_indexes_) :table (table_), @@ -122,7 +122,7 @@ struct hb_prune_langsys_context_t public: const void *table; - hb_hashmap_t *script_langsys_map; + hb_hashmap_t> *script_langsys_map; const hb_map_t *duplicate_feature_map; hb_set_t *new_feature_indexes; @@ -162,14 +162,14 @@ struct hb_subset_layout_context_t : hb_subset_context_t *subset_context; const hb_tag_t table_tag; const hb_map_t *lookup_index_map; - const hb_hashmap_t *script_langsys_map; + const hb_hashmap_t> *script_langsys_map; const hb_map_t *feature_index_map; unsigned cur_script_index; hb_subset_layout_context_t (hb_subset_context_t *c_, hb_tag_t tag_, hb_map_t *lookup_map_, - hb_hashmap_t *script_langsys_map_, + hb_hashmap_t> *script_langsys_map_, hb_map_t *feature_index_map_) : subset_context (c_), table_tag (tag_), @@ -723,12 +723,8 @@ struct Script if (!c->script_langsys_map->has (script_index)) { - hb_set_t* empty_set = hb_set_create (); - if (unlikely (!c->script_langsys_map->set (script_index, empty_set))) - { - hb_set_destroy (empty_set); + if (unlikely (!c->script_langsys_map->set (script_index, hb::unique_ptr {hb_set_create ()}))) return; - } } unsigned langsys_count = get_lang_sys_count (); diff --git a/src/hb-ot-layout-gsubgpos.hh b/src/hb-ot-layout-gsubgpos.hh index b76b43139..63f729c85 100644 --- a/src/hb-ot-layout-gsubgpos.hh +++ b/src/hb-ot-layout-gsubgpos.hh @@ -3729,7 +3729,7 @@ struct GSUBGPOS } void prune_langsys (const hb_map_t *duplicate_feature_map, - hb_hashmap_t *script_langsys_map, + hb_hashmap_t> *script_langsys_map, hb_set_t *new_feature_indexes /* OUT */) const { hb_prune_langsys_context_t c (this, script_langsys_map, duplicate_feature_map, new_feature_indexes); diff --git a/src/hb-subset-plan.cc b/src/hb-subset-plan.cc index a62ae8e02..5bfa2d096 100644 --- a/src/hb-subset-plan.cc +++ b/src/hb-subset-plan.cc @@ -43,7 +43,7 @@ using OT::Layout::GSUB::GSUB; -typedef hb_hashmap_t script_langsys_map; +typedef hb_hashmap_t> script_langsys_map; #ifndef HB_NO_SUBSET_CFF static inline void _add_cff_seac_components (const OT::cff1::accelerator_t &cff, @@ -630,9 +630,6 @@ hb_subset_plan_destroy (hb_subset_plan_t *plan) if (plan->gsub_langsys) { - for (auto _ : plan->gsub_langsys->iter ()) - hb_set_destroy (_.second); - hb_object_destroy (plan->gsub_langsys); plan->gsub_langsys->fini_shallow (); hb_free (plan->gsub_langsys); @@ -640,9 +637,6 @@ hb_subset_plan_destroy (hb_subset_plan_t *plan) if (plan->gpos_langsys) { - for (auto _ : plan->gpos_langsys->iter ()) - hb_set_destroy (_.second); - hb_object_destroy (plan->gpos_langsys); plan->gpos_langsys->fini_shallow (); hb_free (plan->gpos_langsys); diff --git a/src/hb-subset-plan.hh b/src/hb-subset-plan.hh index cb567b769..2aaf19c61 100644 --- a/src/hb-subset-plan.hh +++ b/src/hb-subset-plan.hh @@ -87,8 +87,8 @@ struct hb_subset_plan_t hb_map_t *gpos_lookups; //active langsys we'd like to retain - hb_hashmap_t *gsub_langsys; - hb_hashmap_t *gpos_langsys; + hb_hashmap_t> *gsub_langsys; + hb_hashmap_t> *gpos_langsys; //active features after removing redundant langsys and prune_features hb_map_t *gsub_features; From a42c624fcaa030a68c51acaf007caf402c8c262c Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Fri, 3 Jun 2022 01:22:34 -0600 Subject: [PATCH 19/23] Convert one final use of hashmap to unique_ptr --- src/hb-ot-layout-gsubgpos.hh | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/src/hb-ot-layout-gsubgpos.hh b/src/hb-ot-layout-gsubgpos.hh index 63f729c85..a93d5436d 100644 --- a/src/hb-ot-layout-gsubgpos.hh +++ b/src/hb-ot-layout-gsubgpos.hh @@ -3787,7 +3787,7 @@ struct GSUBGPOS hb_map_t *duplicate_feature_map /* OUT */) const { if (feature_indices->is_empty ()) return; - hb_hashmap_t unique_features; + hb_hashmap_t> unique_features; //find out duplicate features after subset for (unsigned i : feature_indices->iter ()) { @@ -3795,16 +3795,9 @@ struct GSUBGPOS if (t == HB_MAP_VALUE_INVALID) continue; if (!unique_features.has (t)) { - hb_set_t* indices = hb_set_create (); - if (unlikely (indices == hb_set_get_empty () || - !unique_features.set (t, indices))) - { - hb_set_destroy (indices); - for (auto _ : unique_features.iter ()) - hb_set_destroy (_.second); + if (unlikely (!unique_features.set (t, hb::unique_ptr {hb_set_create ()}))) return; - } - if (unique_features.get (t)) + if (unique_features.has (t)) unique_features.get (t)->add (i); duplicate_feature_map->set (i, i); continue; @@ -3849,9 +3842,6 @@ struct GSUBGPOS duplicate_feature_map->set (i, i); } } - - for (auto _ : unique_features.iter ()) - hb_set_destroy (_.second); } void prune_features (const hb_map_t *lookup_indices, /* IN */ From 88f00ecb84a5f78dffabdfa5a8bdc2ed1d452ce4 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Fri, 3 Jun 2022 01:30:27 -0600 Subject: [PATCH 20/23] [map] Fix iter_ref () and test it --- src/hb-map.hh | 4 ++-- src/test-map.cc | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/hb-map.hh b/src/hb-map.hh index f91da642d..cc423c3ef 100644 --- a/src/hb-map.hh +++ b/src/hb-map.hh @@ -94,7 +94,7 @@ struct hb_hashmap_t bool operator == (const K &o) { return hb_deref (key) == hb_deref (o); } bool operator == (const item_t &o) { return *this == o.key; } hb_pair_t get_pair() const { return hb_pair_t (key, value); } - hb_pair_t get_pair_ref() const { return hb_pair_t (key, value); } + hb_pair_t get_pair_ref() const { return hb_pair_t (key, value); } uint32_t total_hash () const { return (hash * 31) + hb_hash (value); } @@ -286,7 +286,7 @@ struct hb_hashmap_t ( + hb_array (items, mask ? mask + 1 : 0) | hb_filter (&item_t::is_real) - | hb_map (&item_t::get_pair) + | hb_map (&item_t::get_pair_ref) ) auto keys () const HB_AUTO_RETURN ( diff --git a/src/test-map.cc b/src/test-map.cc index 9eef49e88..4cb248bd4 100644 --- a/src/test-map.cc +++ b/src/test-map.cc @@ -194,6 +194,7 @@ main (int argc, char **argv) m.get (hb::shared_ptr ()); m.get (hb::shared_ptr (hb_set_get_empty ())); + m.iter (); } /* Test hb::unique_ptr. */ hb_hash (hb::unique_ptr ()); @@ -202,6 +203,7 @@ main (int argc, char **argv) m.get (hb::unique_ptr ()); m.get (hb::unique_ptr (hb_set_get_empty ())); + m.iter_ref (); } return 0; From 9552955e081f3d871765055fd5abad9070cfcf90 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Fri, 3 Jun 2022 01:33:01 -0600 Subject: [PATCH 21/23] Add an unlikely --- src/hb-ot-cmap-table.hh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hb-ot-cmap-table.hh b/src/hb-ot-cmap-table.hh index 9d95f518c..24d548a2a 100644 --- a/src/hb-ot-cmap-table.hh +++ b/src/hb-ot-cmap-table.hh @@ -1485,7 +1485,7 @@ struct SubtableUnicodesCache { hb_set_t* set_for(const EncodingRecord* record) { if (!cached_unicodes.has ((intptr_t) record)) { - if (!cached_unicodes.set ((intptr_t) record, hb::unique_ptr {hb_set_create ()})) + if (unlikely (!cached_unicodes.set ((intptr_t) record, hb::unique_ptr {hb_set_create ()}))) return hb_set_get_empty (); (base+record->subtable).collect_unicodes (cached_unicodes.get ((intptr_t) record)); } From 5dc12d7d8d7aeb3418870d2b3695ff10a53296f6 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Fri, 3 Jun 2022 01:37:02 -0600 Subject: [PATCH 22/23] [cmap] Rewrite set_for() slightly --- src/hb-ot-cmap-table.hh | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/hb-ot-cmap-table.hh b/src/hb-ot-cmap-table.hh index 24d548a2a..4fd2b9be4 100644 --- a/src/hb-ot-cmap-table.hh +++ b/src/hb-ot-cmap-table.hh @@ -1482,12 +1482,20 @@ struct SubtableUnicodesCache { SubtableUnicodesCache(const void* cmap_base) : base(cmap_base), cached_unicodes() {} - hb_set_t* set_for(const EncodingRecord* record) + hb_set_t* set_for (const EncodingRecord* record) { - if (!cached_unicodes.has ((intptr_t) record)) { - if (unlikely (!cached_unicodes.set ((intptr_t) record, hb::unique_ptr {hb_set_create ()}))) + if (!cached_unicodes.has ((intptr_t) record)) + { + hb_set_t *s = hb_set_create (); + if (unlikely (s->in_error ())) + return hb_set_get_empty (); + + (base+record->subtable).collect_unicodes (s); + + if (unlikely (!cached_unicodes.set ((intptr_t) record, hb::unique_ptr {s}))) return hb_set_get_empty (); - (base+record->subtable).collect_unicodes (cached_unicodes.get ((intptr_t) record)); + + return s; } return cached_unicodes.get ((intptr_t) record); } From 215a0afad19a43f88cb8fbeb51877997b40e2567 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Fri, 3 Jun 2022 01:48:46 -0600 Subject: [PATCH 23/23] [algs] Remove unused hb_coerce() --- src/hb-algs.hh | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/hb-algs.hh b/src/hb-algs.hh index ce4849b01..0f08d5070 100644 --- a/src/hb-algs.hh +++ b/src/hb-algs.hh @@ -227,14 +227,6 @@ struct } HB_FUNCOBJ (hb_bool); -template -static inline -constexpr T hb_coerce (const T v) { return v; } -template , hb_decay) && std::is_pointer::value)> -static inline -constexpr T hb_coerce (const V v) { return *v; } - struct { private: