Skip to content
This repository has been archived by the owner on Jun 23, 2022. It is now read-only.

refactor: reimplement join point with less code #711

Merged
merged 3 commits into from
Jan 28, 2021

Conversation

neverchanje
Copy link
Contributor

@neverchanje neverchanje commented Dec 26, 2020

Previously, we have 600+ lines of code for join_point, which is a quite simple class but with a very bloated code.
Now, after this refactoring, it merely has 100+ LOC (including comments).

The interfaces of join_point were mostly unchanged, except for those that don't have any usages. So as you can see, although this class was reimplemented thoroughly, there's no other module affected.

Besides, the original implementation has memory leaks, reported by asan. The current one is leak-free.

template <typename R, typename... Args>
class join_point_base
{
public:
    explicit join_point_base(const char *name) : _name(name) {}

    using ReturnedAdviceT = R(Args...);
    using AdviceT = void(Args...);

    // TODO(wutao): call it add_returned_advice()
    void put_native(std::function<ReturnedAdviceT> fn) { _ret_advice_entries.push_front(fn); }

    // TODO(wutao): call it add_advice()
    void put_back(std::function<AdviceT> fn, const char * /*unused*/)
    {
        _advice_entries.push_back(std::move(fn));
    }

    void put_front(std::function<AdviceT> fn, const char * /*unused*/)
    {
        _advice_entries.push_front(std::move(fn));
    }

    const char *name() const { return _name.c_str(); }

protected:
    std::list<std::function<ReturnedAdviceT>> _ret_advice_entries;
    std::list<std::function<AdviceT>> _advice_entries;
    const std::string _name;

private:
    friend class join_point_test;
};

template <typename R, typename... Args>
class join_point final : public join_point_base<R, Args...>
{
public:
    using BaseType = join_point_base<R, Args...>;
    static_assert(!std::is_void<R>::value, "type R must not be a void");

    explicit join_point(const char *name) : BaseType(name) {}

    R execute(Args... args, R default_return_value)
    {
        R ret = default_return_value;
        for (auto &func : BaseType::_ret_advice_entries) {
            ret = dsn::apply(func, std::make_tuple(std::forward<Args>(args)...));
        }
        for (auto &func : BaseType::_advice_entries) {
            dsn::apply(func, std::make_tuple(std::forward<Args>(args)...));
        }
        return ret;
    }
};

template <typename... Args>
class join_point<void, Args...> final : public join_point_base<void, Args...>
{
public:
    using BaseType = join_point_base<void, Args...>;

    explicit join_point(const char *name) : BaseType(name) {}

    void execute(Args... args)
    {
        for (auto &func : BaseType::_advice_entries) {
            dsn::apply(func, std::make_tuple(std::forward<Args>(args)...));
        }
    }
};

@neverchanje neverchanje added the type/sanitize Fixes on errors reported by sanitizers. label Dec 28, 2020
public:
typedef TReturn (*point_prototype)(T1);
typedef void (*advice_prototype)(T1);
const char *name() const { return _name.c_str(); }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const std::string& name() const { return _name; }

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is what it used to be. I don't wanna change any API.

@acelyc111 acelyc111 merged commit ae16305 into XiaoMi:master Jan 28, 2021
@neverchanje neverchanje deleted the join branch February 2, 2021 05:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type/sanitize Fixes on errors reported by sanitizers.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants