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

remove scheduled functions complexity overhead, change recurrent api #6214

Merged
merged 12 commits into from
Jun 25, 2019

Conversation

d-a-v
Copy link
Collaborator

@d-a-v d-a-v commented Jun 20, 2019

edit:

  • restore simplicity of scheduled function management
  • no overhead when recurrent scheduled function are not used
  • separate API

@d-a-v d-a-v requested review from devyte and earlephilhower June 20, 2019 23:57
Copy link
Collaborator

@earlephilhower earlephilhower left a comment

Choose a reason for hiding this comment

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

LGTM logically. Minor format/content questions, but nothing stopping a merge for me.

cores/esp8266/Schedule.cpp Show resolved Hide resolved
cores/esp8266/Schedule.cpp Outdated Show resolved Hide resolved
cores/esp8266/Schedule.cpp Outdated Show resolved Hide resolved
cores/esp8266/Schedule.cpp Show resolved Hide resolved
cores/esp8266/Schedule.cpp Outdated Show resolved Hide resolved
cores/esp8266/Schedule.cpp Outdated Show resolved Hide resolved
@earlephilhower
Copy link
Collaborator

LGTM.

@dok-net
Copy link
Contributor

dok-net commented Jul 3, 2019

About malloc() vs. new - is this about exceptions when there is no space on heap, or are there other reasons? After all, just reinterpreting uninitialized memory as std::function<void(void)> (the type of member scheduled_fn_t::mFunc) is undefined in the sense of the C++ type system.

According to http://www.cplusplus.com/reference/new/operator%20new/,

new (std::nothrow) MyType

is specified for that purpose.

Besides, this is the code that's actually right in master now at the time of me writing this:

        result = (scheduled_fn_t*)malloc(sizeof(scheduled_fn_t));
        if (result)
        {
            new (&result->mFunc) decltype(result->mFunc)();
            ++sCount;
        }

Could it not rather be?

        result = new (std::nothrow) scheduled_fn_t;
        if (result)
        {
            ++sCount;
        }

@d-a-v d-a-v deleted the recurrent2 branch July 3, 2019 08:30
@devyte
Copy link
Collaborator

devyte commented Jul 5, 2019

@dok-net thanks, the nothrow fix is already done

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