wovo / hwlib

C++ OO micro-controller library for close-to-the-hardware programming
Boost Software License 1.0
57 stars 26 forks source link

Potential lack of keyword final #23

Open Xenoamor opened 5 years ago

Xenoamor commented 5 years ago

https://github.com/wovo/hwlib/blob/23d687944e7ce90c99c9c2b20f113cce3948c3f4/library/pins/hwlib-pin-all.hpp#L63

Looking at this line should the keyword final not be used? If this is omitted then a vtable would need to be created and the generated code would bloat.

I've made an example of this on godbolt

wovo commented 5 years ago

As far as I can see the current version has 'override' on that line, maybe you refere to an older version?

The keyword override in itself doesn't add or remove the need for a vtable: that is cause by the 'virtual' in the supercalss (in this case whlib::pin_out).

Wouter van Ooijen

0638150444 - HL15 4.060


From: Xenoamor notifications@github.com Sent: Sunday, June 30, 2019 2:07 PM To: wovo/hwlib Cc: Subscribed Subject: [wovo/hwlib] Potential lack of keyword final (#23)

https://github.com/wovo/hwlib/blob/23d687944e7ce90c99c9c2b20f113cce3948c3f4/library/pins/hwlib-pin-all.hpp#L63

Looking at this line should the keyword final not be used? If this is committed then a vtable would need to be created and the generated code would bloat.

I've made an example of this on godbolthttps://godbolt.org/z/S7DlGk

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHubhttps://github.com/wovo/hwlib/issues/23?email_source=notifications&email_token=ACE643BT7IKJ6GU7L7K4HXLP5COXHA5CNFSM4H4MNGZKYY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4G4PUJCQ, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ACE643ENA34W273RFDFJG7LP5COXHANCNFSM4H4MNGZA.

Xenoamor commented 5 years ago

Hi Wouter, I think you might have misunderstood me. It is correct to have the keyword override but it would be beneficial to also have the keyword final. If you do then the compiler can de-virtualise it and remove the overheads associated with it. You'll see this optimisation occurring in the godbolt link above also

I'd also like to add that I saw your talk on youtube from a recommendation on Reddit and would like to thank you for it! It's been a great help in selling me on moving to C++

wovo commented 5 years ago

OK, that makes sense. I didn't use final anywhere (yet), but I think there are no disadvantages. But In that case I should probably make the class final.

I'll give it a try (soon).

Wouter van Ooijen

0638150444 - HL15 4.060


From: Xenoamor notifications@github.com Sent: Sunday, June 30, 2019 9:40 PM To: wovo/hwlib Cc: Wouter van Ooijen; Comment Subject: Re: [wovo/hwlib] Potential lack of keyword final (#23)

Hi Wouter, I think you might have misunderstood me. It is correct to have the keyword override but it would be beneficial to also have the keyword final. If you do then the compiler can de-virtualise ithttps://marcofoco.com/the-power-of-devirtualization/ and remove the overheads associated with it

— You are receiving this because you commented. Reply to this email directly, view it on GitHubhttps://github.com/wovo/hwlib/issues/23?email_source=notifications&email_token=ACE643ADZCMAVMDSBK5ESATP5ED4XA5CNFSM4H4MNGZKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODY4SMGI#issuecomment-507061785, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ACE643GA6XZYM5OE2JQ6OT3P5ED4XANCNFSM4H4MNGZA.

Xenoamor commented 5 years ago

Awesome, it'll be interesting to know if it becomes as fast as a templated version. I imagine link time optimisation might be needed for that though

wovo commented 5 years ago

I think even LTO won't optimize as agressively as a template-based version, especially not when multiple ports are used that are passed to multiple functions. IIRC a single port, passed to a single function, is indeed optimized. Tried that, a few years ago.

And with the templated version (godafoss) things are so much easier because not 'inbetween' objects are needed.

Wouter van Ooijen

0638150444 - HL15 4.060


From: Xenoamor notifications@github.com Sent: Sunday, June 30, 2019 9:58 PM To: wovo/hwlib Cc: Wouter van Ooijen; Comment Subject: Re: [wovo/hwlib] Potential lack of keyword final (#23)

Awesome, it'll be interesting to know if it becomes as fast as a templated version. I imagine link time optimisation might be needed for that though

— You are receiving this because you commented. Reply to this email directly, view it on GitHubhttps://github.com/wovo/hwlib/issues/23?email_source=notifications&email_token=ACE643FEGRM75XRXU3I7T43P5EF6TA5CNFSM4H4MNGZKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODY4SVRY#issuecomment-507062983, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ACE643E67VNUPD7SCOBRISLP5EF6TANCNFSM4H4MNGZA.

Xenoamor commented 5 years ago

Yes I see this now, I've played around with it in godbolt and you only get the savings from final if you are directly calling the final class. If you call anything above you get the intermediate objects as you say.

I'll have a look at godafoss when I get some free time!