Cleanup up and Fixing component creation and deletion #2998
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sithhell
force-pushed
the
remove_component_factory
branch
10 times, most recently
from
def8951
to
6157a01
Compare
added 2 commits
November 12, 2017 09:43
- removing operator new/delete overloads - removing functions to retreive component types
sithhell
force-pushed
the
remove_component_factory
branch
from
6157a01
to
0db4a50
Compare
hkaiser
reviewed
Nov 20, 2017
| // | ||
| // return type; | ||
| // } | ||
| // } |
| inline component_type get_component_type() | ||
| { | ||
| // return detail::get_component_type_impl< | ||
| // typename hpx::util::decay<Component>::type>(nullptr); |
| HPX_THROW_EXCEPTION(hpx::unknown_component_address, | ||
| "create<Component>", | ||
| "can't assign global id"); | ||
| void *storage = component_heap<Component>().alloc(1); |
There was a problem hiding this comment.
Does this throw if it fails to allocate memory?
There was a problem hiding this comment.
It might throw. In the case of an exception, it is propagated up to the caller. This is the same behavior as when the Component ctor throws.
| throw; | ||
| return naming::invalid_gid; |
| throw; | ||
| return naming::invalid_gid; |
| { | ||
| HPX_ASSERT(parcels_.size() == handlers_.size()); | ||
| for(std::size_t i = 0; i < parcels_.size(); ++i) | ||
| { | ||
| handlers_[i](e, parcels_[i]); | ||
| handlers_[i].reset(); | ||
| } |
There was a problem hiding this comment.
Is this change related to this PR?
There was a problem hiding this comment.
Not directly... Not sure why I included it. Potentially, it leads to resources being freed in a timelier fashion. I could add it in a different PR if you wish.
| } | ||
| type = derived_component_type(type, base_type); | ||
| // components::enabled(components::get_derived_type(type)) = enabled; | ||
| // components::enabled(components::get_base_type(type)) = enabled; |
| using components::stubs::runtime_support; | ||
| agas::gva g (addr.locality_, addr.type_, 1, addr.address_); | ||
| runtime_support::free_component_sync(g, gid, 1); | ||
| naming::address addr(addr_); |
There was a problem hiding this comment.
The local copy is needed because agas::is_local_address_cached requires an lvalue.
This patch contains various fixes related to component registration, creation
and deletion.
The main motivation was to allow direct creation of components without going
through a factory function, disallowing perfect forwarding for local component
creation.
The component registration has been cleaned up, the factory class for creation
and deletion of components has been removed, and the component type registration
has been moved to the component_registry.
The component deletion has been moved out of the runtime_support server, since
no factory is required anymore.
Various other cleanups have been performed leading to an overall reduction in
binary size and removal of various different duplicate code while preserving
existing functionality.
Fixes include:
- The functionality of component instance counters has been fixed for
local_new
- The deletion of components now correctly takes migrated components
into account
- The component heap instances are now ensured to live in single module only
avoiding freeing memory that has been allocated by a different module.
sithhell
force-pushed
the
remove_component_factory
branch
from
0db4a50
to
a7fc77a
Compare
|
The comments have been addressed. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch contains various fixes related to component registration, creation
and deletion.
The main motivation was to allow direct creation of components without going
through a factory function, disallowing perfect forwarding for local component
creation.
The component registration has been cleaned up, the factory class for creation
and deletion of components has been removed, and the component type registration
has been moved to the component_registry.
The component deletion has been moved out of the runtime_support server, since
no factory is required anymore.
Various other cleanups have been performed leading to an overall reduction in
binary size and removal of various different duplicate code while preserving
existing functionality.
Fixes include:
local_new
into account
avoiding freeing memory that has been allocated by a different module.
Other flyby changes include some cleanup of the runtime_support server and the component interfaces.