Skip to content

Commit a4ee29d

Browse files
capickettfacebook-github-bot
authored andcommitted
Migrate off of RawTable APIs
Summary: The `RawTable` APIs are removed in latest `hashbrown` upgrade (coming in D68793000). rust-lang/hashbrown#546 The PR notes that users should migrate to the `HashTable` APIs instead. - `HashTable`: https://docs.rs/hashbrown/latest/hashbrown/struct.HashTable.html - `RawTable`: https://docs.rs/hashbrown/0.14.5/hashbrown/raw/struct.RawTable.html Reviewed By: dtolnay Differential Revision: D69195912 fbshipit-source-id: b5c4d0cc33a977d9ceb247ffedcf49d7be0b01ec
1 parent 0eb7634 commit a4ee29d

File tree

7 files changed

+66
-131
lines changed

7 files changed

+66
-131
lines changed

allocative/allocative/src/impls/hashbrown.rs

-34
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111

1212
use std::mem;
1313

14-
use hashbrown::raw::RawTable;
1514
use hashbrown::HashTable;
1615

1716
use crate::Allocative;
@@ -20,28 +19,6 @@ use crate::Visitor;
2019

2120
const CAPACITY_NAME: Key = Key::new("capacity");
2221

23-
impl<T: Allocative> Allocative for RawTable<T> {
24-
fn visit<'a, 'b: 'a>(&self, visitor: &'a mut Visitor<'b>) {
25-
use crate::impls::common::DATA_NAME;
26-
use crate::impls::hashbrown_util;
27-
28-
let mut visitor = visitor.enter_self_sized::<Self>();
29-
{
30-
let mut visitor = visitor.enter_unique(DATA_NAME, mem::size_of::<*const T>());
31-
{
32-
let mut visitor = visitor.enter(
33-
CAPACITY_NAME,
34-
hashbrown_util::raw_table_alloc_size_for_len::<T>(self.capacity()),
35-
);
36-
unsafe { visitor.visit_iter(self.iter().map(|e| e.as_ref())) };
37-
visitor.exit();
38-
}
39-
visitor.exit();
40-
}
41-
visitor.exit();
42-
}
43-
}
44-
4522
impl<T: Allocative> Allocative for HashTable<T> {
4623
fn visit<'a, 'b: 'a>(&self, visitor: &'a mut Visitor<'b>) {
4724
use crate::impls::common::DATA_NAME;
@@ -70,7 +47,6 @@ mod tests {
7047
use std::hash::Hash;
7148
use std::hash::Hasher;
7249

73-
use hashbrown::raw::RawTable;
7450
use hashbrown::HashTable;
7551

7652
use crate::golden::golden_test;
@@ -81,16 +57,6 @@ mod tests {
8157
hasher.finish()
8258
}
8359

84-
#[test]
85-
fn test_raw_table() {
86-
let mut table = RawTable::with_capacity(100);
87-
for i in 0..100 {
88-
table.insert(hash(&i.to_string()), i.to_string(), hash);
89-
}
90-
91-
golden_test!(&table);
92-
}
93-
9460
#[test]
9561
fn test_hash_table() {
9662
let mut table = HashTable::with_capacity(100);

app/buck2_interpreter_for_build/src/attrs/coerce/arc_str_interner.rs

+5-5
Original file line numberDiff line numberDiff line change
@@ -11,18 +11,18 @@ use std::cell::RefCell;
1111

1212
use buck2_util::arc_str::ArcStr;
1313
use dupe::Dupe;
14-
use hashbrown::raw::RawTable;
14+
use hashbrown::HashTable;
1515

1616
use crate::attrs::coerce::str_hash::str_hash;
1717

1818
pub(crate) struct ArcStrInterner {
19-
cache: RefCell<RawTable<(u64, ArcStr)>>,
19+
cache: RefCell<HashTable<(u64, ArcStr)>>,
2020
}
2121

2222
impl ArcStrInterner {
2323
pub(crate) fn new() -> ArcStrInterner {
2424
ArcStrInterner {
25-
cache: RefCell::new(RawTable::new()),
25+
cache: RefCell::new(HashTable::new()),
2626
}
2727
}
2828

@@ -34,12 +34,12 @@ impl ArcStrInterner {
3434
let hash = str_hash(s);
3535
let mut cache = self.cache.borrow_mut();
3636

37-
if let Some((_h, v)) = cache.get(hash, |(_h, v)| s == v.as_str()) {
37+
if let Some((_h, v)) = cache.find(hash, |(_h, v)| s == v.as_str()) {
3838
return v.dupe();
3939
}
4040

4141
let value = ArcStr::from(s);
42-
cache.insert(hash, (hash, value.dupe()), |(h, _v)| *h);
42+
cache.insert_unique(hash, (hash, value.dupe()), |(h, _v)| *h);
4343
value
4444
}
4545
}

app/buck2_interpreter_for_build/src/attrs/coerce/interner.rs

+5-8
Original file line numberDiff line numberDiff line change
@@ -18,24 +18,21 @@ use buck2_util::arc_str::ArcSlice;
1818
use buck2_util::arc_str::ArcStr;
1919
use buck2_util::hash::BuckHasher;
2020
use dupe::Dupe;
21-
use hashbrown::raw::RawTable;
21+
use hashbrown::HashTable;
2222

2323
/// An interner specific to our AttrCoercionContext used for interning different kinds of attributes.
2424
/// Things specific about this interner:
2525
/// - Requires interned values to be Dupe, so that you can intern both Arc<...> and specific Arc types like ArcStr.
2626
/// - Interner is not static, so it's not required to take up memory for the entire duration of the program.
2727
pub(crate) struct AttrCoercionInterner<T: Dupe + Hash + Eq, H = BuckHasher> {
28-
/// We use `RawTable` where because `HashMap` API
29-
/// requires either computing hash twice (for get, then for insert) or
30-
/// allocating a key to perform a query using `entry` API.
31-
cache: RefCell<RawTable<(u64, T)>>,
28+
cache: RefCell<HashTable<(u64, T)>>,
3229
_marker: marker::PhantomData<H>,
3330
}
3431

3532
impl<T: Dupe + Hash + Eq, H: Hasher + Default> AttrCoercionInterner<T, H> {
3633
pub(crate) fn new() -> Self {
3734
Self {
38-
cache: RefCell::new(RawTable::new()),
35+
cache: RefCell::new(HashTable::new()),
3936
_marker: marker::PhantomData,
4037
}
4138
}
@@ -49,12 +46,12 @@ impl<T: Dupe + Hash + Eq, H: Hasher + Default> AttrCoercionInterner<T, H> {
4946
let hash = compute_hash(&internable, H::default());
5047
let mut cache = self.cache.borrow_mut();
5148

52-
if let Some((_h, v)) = cache.get(hash, |(_h, v)| internable.equivalent(v)) {
49+
if let Some((_h, v)) = cache.find(hash, |(_h, v)| internable.equivalent(v)) {
5350
return v.dupe();
5451
}
5552

5653
let value: T = internable.into();
57-
cache.insert(hash, (hash, value.dupe()), |(h, _v)| *h);
54+
cache.insert_unique(hash, (hash, value.dupe()), |(h, _v)| *h);
5855
value
5956
}
6057
}

app/buck2_interpreter_for_build/src/interpreter/buckconfig.rs

+7-7
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ use buck2_common::legacy_configs::dice::OpaqueLegacyBuckConfigOnDice;
1717
use buck2_common::legacy_configs::key::BuckconfigKeyRef;
1818
use buck2_core::soft_error;
1919
use dice::DiceComputations;
20-
use hashbrown::raw::RawTable;
20+
use hashbrown::HashTable;
2121
use starlark::collections::Hashed;
2222
use starlark::environment::Module;
2323
use starlark::values::FrozenStringValue;
@@ -46,8 +46,8 @@ struct BuckConfigsInner<'a> {
4646
/// Hash map by `(section, key)` pair, so we do one table lookup per request.
4747
/// So we hash the `key` even if the section does not exist,
4848
/// but this is practically not an issue, because keys usually come with cached hash.
49-
current_cell_cache: RawTable<BuckConfigEntry>,
50-
root_cell_cache: RawTable<BuckConfigEntry>,
49+
current_cell_cache: HashTable<BuckConfigEntry>,
50+
root_cell_cache: HashTable<BuckConfigEntry>,
5151
}
5252

5353
/// Version of cell buckconfig optimized for fast query from `read_config` Starlark function.
@@ -89,8 +89,8 @@ impl<'a> LegacyBuckConfigsForStarlark<'a> {
8989
module,
9090
inner: RefCell::new(BuckConfigsInner {
9191
configs_view,
92-
current_cell_cache: RawTable::new(),
93-
root_cell_cache: RawTable::new(),
92+
current_cell_cache: HashTable::new(),
93+
root_cell_cache: HashTable::new(),
9494
}),
9595
}
9696
}
@@ -115,7 +115,7 @@ impl<'a> LegacyBuckConfigsForStarlark<'a> {
115115
} else {
116116
current_cell_cache
117117
};
118-
if let Some(e) = cache.get(hash, |e| {
118+
if let Some(e) = cache.find(hash, |e| {
119119
e.section.key() == section.key() && e.key.as_str() == *key.key()
120120
}) {
121121
return Ok(e.value);
@@ -134,7 +134,7 @@ impl<'a> LegacyBuckConfigsForStarlark<'a> {
134134
}
135135
.map(|v| self.module.frozen_heap().alloc_str(&v));
136136

137-
cache.insert(
137+
cache.insert_unique(
138138
hash,
139139
BuckConfigEntry {
140140
section: Hashed::new_unchecked(section.hash(), (*section.key()).to_owned()),

starlark-rust/starlark/src/values/trace.rs

-9
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@ use std::sync::Arc;
3535
use std::sync::Mutex;
3636

3737
use either::Either;
38-
use hashbrown::raw::RawTable;
3938
use hashbrown::HashTable;
4039
use starlark_map::small_set::SmallSet;
4140
use starlark_map::Hashed;
@@ -81,14 +80,6 @@ unsafe impl<'v, T: Trace<'v>> Trace<'v> for [T] {
8180
}
8281
}
8382

84-
unsafe impl<'v, T: Trace<'v>> Trace<'v> for RawTable<T> {
85-
fn trace(&mut self, tracer: &Tracer<'v>) {
86-
unsafe {
87-
self.iter().for_each(|e| e.as_mut().trace(tracer));
88-
}
89-
}
90-
}
91-
9283
unsafe impl<'v, T: Trace<'v>> Trace<'v> for HashTable<T> {
9384
fn trace(&mut self, tracer: &Tracer<'v>) {
9485
self.iter_mut().for_each(|e| e.trace(tracer));

starlark-rust/starlark/src/values/types/string/intern/interner.rs

+10-9
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717

1818
//! Generic interner for starlark strings.
1919
20-
use hashbrown::raw::RawTable;
20+
use hashbrown::HashTable;
2121

2222
use crate as starlark;
2323
use crate::collections::Hashed;
@@ -28,7 +28,7 @@ use crate::values::Trace;
2828
/// `[FrozenStringValue]` interner.
2929
#[derive(Default)]
3030
pub(crate) struct FrozenStringValueInterner {
31-
map: RawTable<FrozenStringValue>,
31+
map: HashTable<FrozenStringValue>,
3232
}
3333

3434
impl FrozenStringValueInterner {
@@ -39,14 +39,15 @@ impl FrozenStringValueInterner {
3939
) -> FrozenStringValue {
4040
match self
4141
.map
42-
.get(s.hash().promote(), |x| s == x.get_hashed_str())
42+
.find(s.hash().promote(), |x| s == x.get_hashed_str())
4343
{
4444
Some(frozen_string) => *frozen_string,
4545
None => {
4646
let frozen_string = alloc();
47-
self.map.insert(s.hash().promote(), frozen_string, |x| {
48-
x.get_hash().promote()
49-
});
47+
self.map
48+
.insert_unique(s.hash().promote(), frozen_string, |x| {
49+
x.get_hash().promote()
50+
});
5051
frozen_string
5152
}
5253
}
@@ -55,7 +56,7 @@ impl FrozenStringValueInterner {
5556

5657
#[derive(Default, Trace)]
5758
pub(crate) struct StringValueInterner<'v> {
58-
map: RawTable<StringValue<'v>>,
59+
map: HashTable<StringValue<'v>>,
5960
}
6061

6162
impl<'v> StringValueInterner<'v> {
@@ -66,13 +67,13 @@ impl<'v> StringValueInterner<'v> {
6667
) -> StringValue<'v> {
6768
match self
6869
.map
69-
.get(s.hash().promote(), |x| s == x.get_hashed_str())
70+
.find(s.hash().promote(), |x| s == x.get_hashed_str())
7071
{
7172
Some(string_value) => *string_value,
7273
None => {
7374
let string_value = alloc();
7475
self.map
75-
.insert(s.hash().promote(), string_value, |x| x.get_hash().promote());
76+
.insert_unique(s.hash().promote(), string_value, |x| x.get_hash().promote());
7677
string_value
7778
}
7879
}

0 commit comments

Comments
 (0)