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.

Sign up for GitHub

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

Jump to bottom

Refactoring component_base and base_action/transfer_base_action #3148

Merged

hkaiser merged 1 commit into master from refactor_base_action

Feb 10, 2018

Member

hkaiser commented Feb 6, 2018

This allows to reduce the number of instantiated functions and exported symbols, especially for external modules that contain components.

hkaiser added type: enhancement category: actions category: components labels

hkaiser added this to the 1.1.0 milestone

hkaiser force-pushed the refactor_base_action branch 4 times, most recently from 00cecc6 to 2e8b98d Compare

February 7, 2018 13:17

sithhell approved these changes

View reviewed changes

Member

sithhell left a comment

Just a minor nitpick, looks good otherwise!

src/runtime/components/server/component_base.cpp Outdated

+                  naming::address component_base::get_current_address() const
+                  {
+                      return naming::address(get_locality(), type_,
+                          std::uint64_t(static_cast<component_base const*>(this)));

Member

sithhell Feb 7, 2018

the static cast here is not necessary, is it?

Member Author

hkaiser Feb 7, 2018

It is needed as this might be different from the base address of the whole component.

Member Author

hkaiser Feb 7, 2018

But this code is wrong for other reasons, will fix.

hkaiser force-pushed the refactor_base_action branch from 2e8b98d to 881ad18 Compare

February 7, 2018 15:13

sithhell reviewed

View reviewed changes

hpx/runtime/components/component_type.hpp Outdated

    
            @@ -174,12 +174,12 @@ namespace hpx { namespace components
          
                  // Returns the (unique) name for a given component

                  template <typename Component, typename Enable = void>

                  const char* get_component_name();

                  HPX_CONSTEXPR char const* const get_component_name();

Member

sithhell Feb 7, 2018

the extra const generates warnings for clang.

hpx/runtime/components/component_type.hpp Outdated

                   // Returns the (unique) name of the base component. If there is none,
                   // nullptr is returned
                   template <typename Component, typename Enable = void>
-                  const char* get_component_base_name();
+                  HPX_CONSTEXPR const char* const get_component_base_name();

Member

sithhell Feb 7, 2018

same here.

hpx/runtime/components/component_type.hpp Outdated

    
                  template <> HPX_ALWAYS_EXPORT                                             \

                  const char* get_component_name< Component, void>()                        \

                  template <> HPX_CONSTEXPR                                                 \

                  char const* const get_component_name< Component, void>()                  \

Member

sithhell Feb 7, 2018

same here.

hpx/runtime/components/component_type.hpp Outdated

    
                  template <> HPX_ALWAYS_EXPORT                                             \

                  const char* get_component_base_name< Component, void>()                   \

                  template <> HPX_CONSTEXPR                                                 \

                  char const* const get_component_base_name< Component, void>()             \

Member

sithhell Feb 7, 2018

same here.

hpx/runtime/components/component_type.hpp Outdated

    
                  template <> HPX_ALWAYS_EXPORT                                             \

                  const char* get_component_name< Component, void>()                        \

                  template <> HPX_CONSTEXPR                                                 \

                  char const* const get_component_name< Component, void>()                  \

Member

sithhell Feb 7, 2018

same here.

hpx/runtime/components/component_type.hpp Outdated

    
                  template <> HPX_ALWAYS_EXPORT                                             \

                  const char* get_component_base_name< Component, void>()                   \

                  template <> HPX_CONSTEXPR                                                 \

                  char const* const get_component_base_name< Component, void>()             \

Member

sithhell Feb 7, 2018

same here.

hkaiser force-pushed the refactor_base_action branch 7 times, most recently from c0673fc to 7e4cabf Compare

February 8, 2018 15:02

Member

sithhell commented Feb 9, 2018

appveyor is not happy with this right now

Member Author

hkaiser commented Feb 9, 2018

appveyor is not happy with this right now

Yes, I know - but this was introduced earlier by one of the changes to the thread scheduling.

Member

sithhell commented Feb 9, 2018

Yes, I know - but this was introduced earlier by one of the changes to the thread scheduling.

Can you restart the build then? It looks like this PR is the only one affected to that timeout.

Member Author

hkaiser commented Feb 9, 2018

The same happened here: https://ci.appveyor.com/project/hkaiser/hpx/build/fix_timed_suspension-1897, apparently this got branched off at a bad time... I'll rebase and restart the tests.


          Refactoring component_base and base_action/transfer_base_action to re…

…duce number of instantiated functions and exported symbols

 - making get_component_type() related functionality constexpr (instead of always exporting the functions)

hkaiser force-pushed the refactor_base_action branch from 7e4cabf to 2783320 Compare

February 9, 2018 16:10

hkaiser merged commit c45c82e into master

hkaiser deleted the refactor_base_action branch

February 10, 2018 01:39

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment