On Fri, Oct 29, 2021 at 08:16:08AM +0100, Sean Young wrote:
> On Thu, Oct 28, 2021 at 08:05:16PM +0200, Uwe Kleine-König wrote:
> > On Thu, Oct 28, 2021 at 01:26:10PM +0100, Sean Young wrote:
> > > > bloat-o-meter reports (for an arm allmodconfig build)
> > > >
> > > > add/remove: 0/0 grow/shrink: 3/1 up/down: 644/-396 (248)
> > > > Function old new delta
> > > > pwm_ir_probe 372 676 +304
> > > > pwm_ir_set_carrier 108 292 +184
> > > > pwm_ir_set_duty_cycle 68 224 +156
> > > > pwm_ir_tx 908 512 -396
> > > > Total: Before=2302, After=2550, chg +10.77%
> > >
> > > So 248 bytes more after your changes.
> >
> > ack. This is because the compiler inlines the division which accounts
> > for > 100 bytes.
>
> I'm surprised it's that large. This is on 32 bit?
Yes, it's a 64 bit division on 32 bit ARM.
> > > > struct pwm_ir increases from 12 bytes to 40 bytes.
> > > >
> > > > The stack space required by pwm_ir_tx decreases from 60 to 36
> > > >
> > > > I don't know exactly how kmalloc works internally. Maybe allocating a
> > > > structure of size 40 bytes doesn't need more memory than a structure of
> > > > size 12?
> > > >
> > > > I didn't check how runtimes change, but the size decrease of pwm_ir_tx()
> > > > is nice and might save a bit of runtime.
> > >
> > > I'm not following, how is this decreasing runtime?
> >
> > With my changes pwm_ir_tx got smaller and { pwm_ir_probe,
> > pwm_ir_set_carrier, pwm_ir_set_duty_cycle } got bigger. Now if for a
> > typical runtime pattern pwm_ir_probe and pwm_ir_set_carrier run once and
> > pwm_ir_set_duty_cycle 100 times and pwm_ir_tx 1000 times (no idea if
> > that is realistic) it might be a net win in sum.
>
> The two most common programs for sending IR are
>
> ir-ctl: https://git.linuxtv.org/v4l-utils.git/tree/utils/ir-ctl/ir-ctl.c#n1041
> lircd: https://sourceforge.net/p/lirc/git/ci/master/tree/lib/transmit.c
>
> For each transmission, the carrier is set. If the duty cyle is specified,
> then that is set too. Then the transmit itself is done. Both of them
> set the carrier and duty cycle (if required) for every transmission: setting
> the carrier and duty cycle is a cheap operation, and it is device property
> which can be overriden by another process.
>
> This means with your changes, if the carrier and duty cycle are both set
> for each transmission, then we're doing more work. If only the carrier
> is set for each transmission, then there is no net gain/loss (I think),
> but the code size has increased.
OK, then I discard my patch.
While reading that I wondered if it makes sense to have a callback that
sets both carrier and duty cycle and then remove the other two.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
Attachment:
signature.asc
Description: PGP signature
