Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RHashMap::raw_entry[_mut] support #83

Open
wants to merge 627 commits into
base: 0_11_old
Choose a base branch
from

Conversation

marioortizmanero
Copy link
Contributor

@marioortizmanero marioortizmanero commented Mar 6, 2022

This PR adds support for the upcoming raw_entry API. This is possible by switching to hashbrown instead of std. Furthermore, this PR enables support for halfbrown, an empirically more performant implementation of hashbrown, with support for raw_entry, too:

Hashmap implementation that dynamically switches from a vector based backend to a hashbrown based backend as the number of keys grows

The halfbrown part works by switching the underlying struct for RHashMap to a halfmap::HashMap when activated. Its interface is meant to be the same as hashbrown's.

The only error I haven't been able to fix is the following, any help?

error[E0271]: type mismatch resolving `<iterator_stuff::RefIterInterface<K, V> as erased_types::traits::InterfaceType>::Clone == impl_enum::Unimplemented<type_leve
l::trait_marker::Clone>`
   --> abi_stable/src/std_types/map/extern_fns.rs:123:13
    |
123 |             DynTrait::from_borrowing_value(iter, RefIterInterface::NEW)
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected struct `impl_enum::Unimplemented`, found struct `impl_enum::Implemented`
    |
    = note: expected struct `impl_enum::Unimplemented<type_level::trait_marker::Clone>`
               found struct `impl_enum::Implemented<type_level::trait_marker::Clone>`
note: required because of the requirements on the impl of `erased_types::vtable::GetVtable<'_, std::iter::Map<halfbrown::Iter<'_, map_key::MapKey<K>, V>, fn((&map_
key::MapKey<K>, &V)) -> tuple::Tuple2<&K, &V> {map_iter_ref::<'_, K, &V>}>, std_types::boxed::private::RBox<()>, std_types::boxed::private::RBox<std::iter::Map<hal
fbrown::Iter<'_, map_key::MapKey<K>, V>, fn((&map_key::MapKey<K>, &V)) -> tuple::Tuple2<&K, &V> {map_iter_ref::<'_, K, &V>}>>, iterator_stuff::RefIterInterface<K,
V>>` for `interface_for::InterfaceFor<std::iter::Map<halfbrown::Iter<'_, map_key::MapKey<K>, V>, fn((&map_key::MapKey<K>, &V)) -> tuple::Tuple2<&K, &V> {map_iter_r
ef::<'_, K, &V>}>, iterator_stuff::RefIterInterface<K, V>, downcasting::TD_Opaque>`
   --> abi_stable/src/erased_types/vtable.rs:333:13
    |
333 |               GetVtable<'borr,$value,$erased_ptr,$orig_ptr,$interf>
    |               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
334 |           for This
    |               ^^^^
...
495 |   declare_meta_vtable! {
    |  _-
    | |_|
    | |
496 | |     interface=I;
497 | |     value  =T;
498 | |     erased_pointer=ErasedPtr;
...   |
757 | |     ]
758 | | }
    | | -
    | |_|
    | |_in this macro invocation
    |   in this macro invocation
note: required by a bound in `priv_::DynTrait::<'static, rref::RRef<'static, ()>, ()>::from_borrowing_value`
   --> abi_stable/src/erased_types/dyn_trait.rs:877:44
    |
870 |         pub fn from_borrowing_value<'borr, T, I>(
    |                -------------------- required by a bound in this
...
877 |             InterfaceFor<T, I, TD_Opaque>: GetVtable<'borr, T, RBox<()>, RBox<T>, I>,
    |                                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `priv_::DynTrait::<'static, rref::RRef<'static
, ()>, ()>::from_borrowing_value`
    = note: this error originates in the macro `declare_meta_vtable` (in Nightly builds, run with -Z macro-backtrace for more info)

I understand that the way to fix it would be to remove Clone from here:

declare_iter_interface! {
    K => V;
    /// The `InterfaceType` of the `Iter` iterator for `RHashMap`.
    #[sabi(impl_InterfaceType(Iterator, Clone))]
    interface = RefIterInterface;
    type Item = Tuple2<&'a K, &'a V>;
}

But then I get errors, because .clone() is actually used a few times.

TODO:

  • Fix error above
  • Disable hashbrown when halfbrown is being used? That would mean hashbrown would also be a (default) feature. But if someone uses --no-default-features it would fail without also adding hashbrown back.
  • Update the tests and the docs
  • Implement "nightly" features for halfbrown/hashbrown:
    • insert_nocheck
    • raw_entry
  • Wait for Fix hasher #85 and rebase (this branch points to it right now, so the diff is a bit messed up)

Removed all the code that was only compiled before 1.39.

Made `RSliceMut::from_raw_parts_mut` a const fn.

Made `take_manuallydrop` use `ManuallyDrop::take` from Rust 1.42
Replaced all uses of `StaticStr` with `RStr<'static>`,
and `StaticSlice<T>` with `RSlice<'static,T>`

Changed the `version_compatibility/interface` crate to not require a feature for `old`,
but still error if both the "old" and "new" features are used.
Bumped as_derive_utils dependency to 0.8.3
Only made sure that abi_stable and abi_stable_derive compile,and the tests pass.
`abi_stable::library` is commented out, that will be fixed in a later commit.

Declared the PrefixRef type, a non-nullable pointer for all prefix types.
This type replaces all instances of the `&Prefix` and `StaticRef<Prefix>`.

Now prefix don't work using references to the prefix,
instead there's a `PrefixRef` newtype over a NonNull,
which is the only way to access the prefix.
And every type has a `DerivingType_Ref` type (newtype around a `PrefixRef`),
which defines the accessor methods for the fields in `DerivingType`

Now the `#[sabi(kind(Prefix))]` attribute takes 2 optional arguments:
- `prefix_ref`(defaults to `<DerivingType>_Ref`):
    The name of the generated pointer type, with accessor methods for
    the fields of the deriving type (some of which are conditional).
- `prefix_fields`(defaults to `<DerivingType>_Prefix`):
    The name of the generated prefix struct,
    with only the fields up to(and including) the #[sabi(last_prefix_field)]` attribute.

Removed the `*Val` suffix for types deriving StableAbi with a prefix kind.

Updated the tests to compile and pass.

Updated the doctests to pass, and changed some tests to
use a `const` instead of leaking to construct the pointer to prefix.

Replaced virtually all uses of StaticRef with PrefixRef (or newtypes that wrap it),
since all the replaced uses were for prefix types.
Changed StaticRef to use NonNull, so that Option<StaticRef> is ffi-safe.
Updated the StaticRef example to be for a non-extensible vtable.

Removed the Kind associated type from StableAbi.

Split off SharedStableAbi trait from StableAbi, now there's PrefixStableAbi and StableAbi,
both of which only have StaticEquivalent_ in common.

Added NonNullPointer for later use in generalizing LateStaticRef to
support any type wrapping a non-nullable pointer
(including the `DerivingType_Ref` types).

Replaced WithMetadataFor with the new PrefixMetadata,
with the type layout for prefix field error messages.
Defined accessor methods for PrefixMetadata's fields.
And storing PrefixMetadata as the first field of WithMetadata_.

Defined WithMetadata_ methods for returning a `PrefixRef`,removing the StaticRef ones.

Removed PrefixTypeTrait::into_with_metadata method,added PrefixFields and PrefixRef assoc types.

Made the type parameters in the vtable of `#[sabi_trait]` trait objects covariant.

The <DerivingType>_Ref type implements the Copy,Clone,GetStaticEquivalent_,StableAbi,
NonNullPointer traits

Changed how prefix type accessor methods work,
fields after the suffix are accessed by offsetting a raw pointer to the offset of the field.
The field offsets are computed at compile-time,using a function from the `repr_orrset` crate,

Added `repr_offset` as a dependency of `abi_stable_derive`
The `staticref` macro was defined with a non-trivial example.

Modified examples that construct vtables to use the `staticref` macro,
removing an `unsafe` for each replacement.

Added `pmr` as an alias for the `abi_stable::derive_macro_reexports` module.

Improved the docuementation example for StaticRef.

Added `PrefixRef::leak_value` constructor.

Renamed the `PrefixRef::new` constructor to `PrefixRef::from_raw`
Added the `PrefixRef::{from_staticref, from_ref}` safe constructors
LateStaticRef is now generic over the returned pointer,
so it supports references and prefix pointers.

Changed uses of `LateStaticRef<T>` to `LateStaticRef<&T>`

Fixed an unsoundness bug where `LateStaticRef<T>` implemented Send + Sync
even if `T` didn't, this was because AtomicPtr always implements Send + Sync .

Renamed `LateStaticRef::<&T>::initialized` to `from_ref`.

Added an unsafe `LateStaticRef::from_custom` constructor for non-`&` types.

Added Send + Sync supertraits to TypeChecker and ExtraChecks

Renamed NonNullPointer to ImmutableRef,to reflect how it's intended only for pointer types
that always point to valid initialized data, but don't necessarily implement `Deref`.

Added `ImmutableRef::{to_raw_ptr, from_raw_ptr}` methods to convert to
and from raw pointers.

Made all `ImmutableRef` methods defaulted.

Defined `utils::Transmuter` to transmute between Copy types without overhead in debug builds.

Defined the NonNullTarget marker type,and `ImmutableRef::TARGET` that construct it.

Added `PrefixRef::as_nonnull` method.
Uncommented the `abi_stable::library` module, and  updated it to compile after the
prefix type rewrite.

Now the RootModule trait also has a `PrefixRefTrait` supertrait.
Replaced every instance of `&'static M` (where `M` is a `RootModule`) with `M`.
LibHeader stores a `PrefixRef<ErasedPrefix>` insetad of `&'static ErasedObject`.

Added `prefix_type::PrefixRefTrait` trait for ffi-safe pointers to prefix types.
Added `prefix_type::GetWithMetadata:` as a helper for `PrefixRefTrait`.

Added `marker_type::ErasedPrefix` marker type, similar to `ErasedObject` but for prefix fields.

Added the `LateStaticRef::<PrefixRef<P>>::from_prefixref` constructor.

Fixed a soundness(?) bug where the `RBpxErrpr` returned from checking the layout
of a library can contain references into the unloaded library,
by stringifying the errors into a new `RBoxError`,

Added a `RBorError_::to_formatted_error` pub method, as_debug_display private method,
and `as_debug_display` vtable entry.

Added tests for the new RBoxError_ features.
Updated `export_root_module` attribute to the new style of prefix types.

Updated all the crates in the workspace to use the new style of prefix types,
so that they compile.

Made all the tests in the workspace pass(more details below).

Fixed the ron-file-based tests for StableAbi to use the new kind of prefix types

Fixed `LateStaticRef::from_prefixref` to allow using any `PrefixRefTrait` type, fixed its doctests.

Added example for `LateStaticRef::from_custom`

Renamed ImmutableRefTarget type alias to ImmutableRefOut
Renamed NonNullTarget struct to ImmutableRefTarget

Added the `PointsToPrefixFields` marker type,
and `PrefixRefTrait::PREFIX_FIELDS` associated constant to construct it.

Implemented PrefixRefTrait for PrefixRef

Added the `utils::ref_as_nonnull` safe function, to convert references to NonNull.

Added a branch to the `declare_root_module_statics` macro to safeguard against passing `Self`.

Temporarily disabled the `old_stable_abi` dependency in version tests,
remember to reenable after uploading the 0.9.0!!!
Updated the docs in `abi_stable::docs::prefix_types`

Updated the docs for `#[sabi(...)]` attribute to include the changes
made to the `#[sabi(kind(Prefix(....)))]` attribute.

Added docs with examples to `PrefixRef`

Split `PrefixRef::as_ptr` method into `to_raw_ptr` (called by runtime function),
and `const_to_raw_ptr` (only called in const functions)

Removed the `PrefixRef::as_nonnull` inherent method.

Made the fields in the prefix have the same visibility as in the deriving type,
except for conditional field which are always private.

Added ron-based test for the names of the generated structs when using the
`#[sabi(kind(Prefix(....)))]` with different sets of arguments.

Defined the `abi_stable::for_examples` module, with types used in documentation examples.
Added test to check that `*_Prefix` structs make all conditional fields private.

Added test to check that `*_Ref` accessors work correctly with suffix fields that have
different alignments.

Added test to check that `#[sabi_trait]` allows defining defaulted methods in the prefix
(any method with or above the `#[sabi(last_prefix_field)]` attribute).
Replaced all uses of `PhantomData<fn()->T>` with `NonOwningPhantom<T>`

Replaced all uses of `PhantomData<TupleN<T0, T1, T...>>` with `PhantomData<(T0, T1, T...)>`

Fixed(?) the variance of type parameters in sabi_trait generated trait object to be covariant.

Removed redundant `Transmuter` union and `AssertEq` struct declarations.
Added crates in `/testing/1 - loading errors/` to test the errors that happen while
loading a dynamic library, starting with abi incompatibility errors.

Moved the `compute_library_path` function in a bunch of the "user" crates to the newly created
`abi_stable::library::development_utils` module,
made the function take the path to the target and the RootModule type as parameters.

Moved the `testing/*_0` crates to the new `testing/0/` directory.

Updated the travis configuration (will probably have to fix it when CI runs),

Bumping the minimum supported rust version, update the readme and in CI tests.
Updated the `#[sabi(last_prefix_field)]` attribute on a few types and traits.

Made MovePtr,NulStr, and RRef non nullable.
This means that `Option<NulStr>` for example now implements StableAbi, when it didn't before.

Added test ensuring that non-null pointers all implement StableAbi,
an Option wrapping them also implement StableAbi,
as well as their layout being the same wrapped and unwrapped in Option.
The root module loader function can now return module,`Result<module, RBoxError>`,
or any other type implementing IntoRootModuleResult.

Moved LibraryError to the new `abi_stable::library::errors` module.

Declared the `RootModuleError` type and `LibraryError::RootModule` variant
for the errors returned by root module loaders.

Declared the `IntoRootModuleResult` trait to convert the return value of
`#[export_root_module]` functions

Moved some of the generated code for the `#[export_root_module]` attribute to the
`abi_stable::library::__call_root_module_loader` function

Declared the `abi_stable::library::RootModuleResult` `#[doc(hidden)]` type alias,
for the return type of the wrapping function created by the `#[export_root_module]` attribute

Updated the LibHeader code that handles the return value of the root module loader function.

Improved the docs of the RootModule trait a bit.

Updated the docs for the `#[export_root_module]` attribute.

Modified the "testing/1 - loading errors/" crates to test fallible module loaders that do these:
- Return the module
- Return an error.
- Panic.

Updated the travis config.

Added a RootModule impl to for_examples::Module_Ref for use in tests.

Defined a test for `IntoRootModuleResult::into_root_module_result`

Changed `RBox::{from_fmt, from_debug, to_formatted_error}` to take references.
Now the derive macro rejects `#[repr(C, packed)]` prefix types.
I don't feel like supporting those,don't think anyone needs packed vtables/modules

Added test accessing fields in the a prefix struct that is overaligned,
and with even more overaligned fields.

Fixed an error from `ManuallyDrop::new` being `#[must_use]`
Made a bunch of items `#[doc(hidden)] pub` to use from the `abi_stable/tests` folder.
Fixed `RMutex::get_mut`, which caused a memory leak.

Fixed `RRwLock::get_mut`, which caused a memory leak.

Overrode ImmutableRef::{to_raw_ptr, from_raw_ptr} for references to appease miri.

Removed all the remaining intentional memory leaks in tests.
Defined the RMut type for use as the self parameter in the implementation of the trait objects.

Changed the implementation of the trait objects to use
RRef for &self, RMut for &mut self, and *mut () for self.

Added sabi_as_rref and sabi_as_rmut methods to RObject and DynTrait to get RRef and RMut.
Calling the `miri_static_root` function to tell miri that the memory I leak intentionally
is actually used.

Extracted a more minimal version of many layout tests,to run in a reasonable amount of time in miri.
This means that there's versions of those functions for miri and not.

Updated travis config to run miri tests, hopefully this won't run out of RAM.
Added micro-optimization where `#[sabi_trait]` changes how the ffi-safe vtable implementation
returns the return value of calling the trait method.

Added tests for the new optimization.
Bumped minimum supported Rust version to 1.40.0

Removed mentions of how ROnce can't be put in a static because it can now.
@marioortizmanero
Copy link
Contributor Author

Here's an update about the status of this PR.

I have concluded my work with Tremor, so I won't be able to continue working on it. For now, the PR is just part of research in trying to optimize Tremor's plugin system. It analyzes halfbrown vs hashbrown, and memoizing JSON hashes (see https://nullderef.com/blog/plugin-end/#_hash_table_optimization). If the rest of the Tremor team end up considering this worth implementing, then some more polishing is still required. The wrapped API is incomplete, and doesn't include the builder pattern for simplicity.

Even if the change ends up being worth it, I'm not fully sold on merging it into the main branch. raw_entry isn’t going to make it to a stable release, and the Rust team is working on a different interface. It will probably be similar enough that updating our raw_entry usage won’t be too much work, but we should make sure first to minimize the breaking changes.

Any more doubts please let me know!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants