Skip to content

New Delegate class to boost performance of callbacks previously using std::function #6857

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

Open
wants to merge 52 commits into
base: master
Choose a base branch
from

Conversation

dok-net
Copy link
Contributor

@dok-net dok-net commented Nov 28, 2019

The Delegate class can perform calls at C-function pointer with void* argument pair efficiency, while offering the flexibility to simply use lambdas with captures and std::bind in the same places without changing the API.

void readAsync(Delegate<void,void*,int> cb)
{
    …
    cb(recvCount);
    …
}
…
readAsync([this](int received) { this->receivedAsync(received) });

or

static void receivedAsync(void* self, int received)
{
    ((SomeClass*)self)->receivedAsync(received);
}
...
readAsync({ receivedAsync, this });

The ISR handling PR #6047, Schedule.h|cpp adaptation, and WiFi etc. callbacks PR #6782 are updated and waiting to be published.

@dok-net dok-net force-pushed the delegate branch 4 times, most recently from 5570305 to 9f14b1d Compare December 3, 2019 15:55
@dok-net dok-net changed the title New Delegate class to boost performance of callbacks using std::function New Delegate class to boost performance of callbacks previously using std::function Dec 4, 2019
@dok-net
Copy link
Contributor Author

dok-net commented Dec 13, 2019

@earlephilhower It seems that you are the one taking care of CI. This build is fine except the Mac target has trouble d/l-ing the compiler suite or something at Travis. Is there any user-available way that I could retrigger failed builds without pushing changes to GitHub?

@earlephilhower
Copy link
Collaborator

@dok-net, looked like a temporary network interruption. I'm not sure if the submitter can do it, but next time you can hit the Details link, then on the RHS of the list of Travis-CI jobs there should be a small button you can use to re-run a job. If it's not there, then only the maintainers can and you'll have to wait. Don't sweat a single job failure, we can always re-fire just that job while reviewing code.

@dok-net dok-net force-pushed the delegate branch 3 times, most recently from 6656bcc to afd76b1 Compare December 20, 2019 20:36
@dok-net dok-net force-pushed the delegate branch 3 times, most recently from d41bd7a to 294ecaa Compare December 24, 2019 22:48
@dok-net dok-net force-pushed the delegate branch 3 times, most recently from 82dd50f to befbd7e Compare January 22, 2020 00:08
@dok-net dok-net force-pushed the delegate branch 2 times, most recently from 298899a to 20ace81 Compare February 5, 2020 06:27
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.

I assume this has the same issue with C++17 and implicit copy constructors as the version in SoftwareSerial. plerup/espsoftwareserial#140

Also, these add'l 2,362 lines code don't seem to be used for anything in the core with this PR. Maybe a separate library is the way to go? At the very least some sort of documentation/examples since the only use would be end users' code. OTW it's not really visible to the users.

@dok-net
Copy link
Contributor Author

dok-net commented Feb 26, 2020

Once you move on to some of the other pending PRs of mine, you'll find that Delegate and MultiDelegate do get used a lot in core.
Switching to gcc 9.2 and enabling a very fresh deprecation warning, plus turning it into an error, is adding quite a bit pressure on the poor developer having to port their code... I've been able to verify with MSVC in C++17 mode, and GCC-8 until now, and I've added an academic fix for the -Wdeprecated-copy related situation. I'm still needing time to update to a desktop Linux distribution that already includes GCC 9.2. If you get around to running the build process for ESP8266 ahead of me, please let me know the outcome. The latest fixes are in the master branches of the various repositories I manage (https://github.com/plerup/espsoftwareserial).

@dok-net
Copy link
Contributor Author

dok-net commented Feb 28, 2020

@earlephilhower I've got it building on 64bit Linux now. GCC 9.2 is a serious pain w.r.t. to that warning, I had to copy and check ctors and operator=s without end from one template to another to another, just because it's the only compiler that refuses to infer types easily and allow "using" on ctors and assignment ops without balking. Btw, it's not about C++ 17, GCC 9.2 complains the same for the other C++ releases :-(

@earlephilhower
Copy link
Collaborator

Thanks for the info. I thought the deprecation was related to a C++ standard, but oh well. The version of Delegate.h in SoftwareSerial looks to be building just fine now, thx!

@dok-net
Copy link
Contributor Author

dok-net commented Feb 28, 2020 via email

dok-net added 29 commits June 28, 2021 12:48
…ecializations. Portability fix for Non-ESP Arduino.
…ATTR attribute.

This affects the unnamed namespace or static non-member functions.
…uto-remove. Invoke returns result of last Delegate call.

Caveat: breaks use of event multiplexer with auto-remove; WIP: add iterators.
…ent operators, needing a lot

of code duplication, this commit provides that.
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.

3 participants