Open Bug 1557617 Opened 5 years ago Updated 2 years ago

HashMap's inflexible alignof restriction seems arbitrary

Categories

(Core :: MFBT, enhancement, P3)

enhancement

Tracking

()

Tracking Status
firefox69 --- affected

People

(Reporter: mozbugz, Unassigned)

Details

Bug 1508873 reduced the wasted space when storing some kinds of types, but also imposed a strict limit on the alignment of entries (and hence on stored types).

For example, this prevents HashMap<int, std::function<void(void)>>, because std::function has an alignment of 16 on macos (the restriction is 8 there).

If someone really needed a map to std::function, they would have to use other map types (e.g., std::map works) that may still end up with the same (or worse) waste of space; so adding the restriction in HashMap doesn't really help I think.

Now, I see the value in highlighting that some HashMap is sub-optimized, so that the developer may know about it and may try to improve the situation. But in the end, I think there should be a way to stick with HashMap instead of being forced into using another type. So I won't request that we just remove this restriction.

Instead I would like to suggest that we add an "escape hatch" of some kind, so that the diligent developer will still be warned about the waste of space, but will have the option to go ahead anyway.

E.g., there could be an extra template argument that removes this restriction: HashTable<int, function, DefaultHasher<int>, MallocAllocPolicy, THIS_MAP_IS_WASTING_SPACE>. It's ugly, but that's the point, it should be highly visible.
(There could be other ways, that's just one suggestion.)

It's not "sub-optimized", it's that HashMap is not set up to handle entries with alignment greater than 8. We'd need to posix_memalign the memory or something like that, and even then I think we have a maximum alignment of entries.

Type: defect → enhancement
Keywords: regression
Priority: -- → P3
No longer regressed by: 1508873
Severity: normal normal → S3 S3
You need to log in before you can comment on or make changes to this bug.