[BY DESIGN] Slow response to external ISR when using AttachInterrupt

LibMaple (The core that Roger's repo uses)
User avatar
RogerClark
Posts: 6692
Joined: Mon Apr 27, 2015 10:36 am
Location: Melbourne, Australia
Contact:

Re: Slow response to external ISR when using AttachInterrupt

Post by RogerClark » Fri Aug 25, 2017 11:55 am

Using Pito's test program, I tried 2 different sets of pins, and for pins where the EXTIs do not share an IRQ, the response time was 500nS, which is almost good as @buaus gets using IAR

However when EXTIs share an ISR, its much slower at around 1679nS on my Maple Mini

If I modify the code and add a dummyHandler() which is the default for all EXTI's, the code can be changed to remove one "if" statement and also one variable declaration, and this reduces the time by around 100nS

If I turn up the optimisation to -O3 the time reduces to 958 nS

-O3 with LTO makes things slower

Note. I get warnings when compiling on -O3 which probably need to be investigated - but this is nothing to do with the interrupt latency

I think things could be speeded up a bit more my unwinding the for loop, as the dispatch_extis function is only ever called from 2 places

void __irq_exti9_5(void) {
dispatch_extis(5, 9);
}

void __irq_exti15_10(void) {
dispatch_extis(10, 15);
}


the code could be moved to the calling function, and the bit patterns could be pre-calculated, so that this shift and store could be removed

uint32 eb = (1U << exti);


However, I don't think we would get a massive speed improvement, so its probably not worth the increase in code size

User avatar
Pito
Posts: 1498
Joined: Sat Mar 26, 2016 3:26 pm
Location: Rapa Nui

Re: Slow response to external ISR when using AttachInterrupt

Post by Pito » Fri Aug 25, 2017 12:42 pm

With Pito's code and STM32Generic (SerialUSB, default opt, MapleMini) after a minute:

Code: Select all

INTR Latency MIN=3262ns MAX=26205ns AVER=3343ns
INTR Latency MIN=3262ns MAX=26205ns AVER=3343ns
..
Last edited by Pito on Fri Aug 25, 2017 3:40 pm, edited 2 times in total.
Pukao Hats Cleaning Services Ltd.

User avatar
RogerClark
Posts: 6692
Joined: Mon Apr 27, 2015 10:36 am
Location: Melbourne, Australia
Contact:

Re: Slow response to external ISR when using AttachInterrupt

Post by RogerClark » Fri Aug 25, 2017 12:44 pm

Ummm

I wonder what is making it so slow

stevestrong
Posts: 1511
Joined: Mon Oct 19, 2015 12:06 am
Location: Munich, Germany

Re: Slow response to external ISR when using AttachInterrupt

Post by stevestrong » Mon Aug 28, 2017 2:17 pm

RogerClark wrote:
Thu Aug 24, 2017 11:56 pm
If all the handlers were initially set to go to a dummy function, there would be no need to check whether the handler was valid

This would save 1 "if" function, and "if"s seem to be where ARM code gets slowed down
Additionally, the passed pointer to arguments seems to be unused, so it could be removed (exti_channels[exti].arg = not needed).
See here: NULL parameter passed: https://github.com/rogerclarkmelbourne/ ... exti.c#L94
So the table would reduce to one column containing only the void function pointers.
This would save RAM, FLASH and run-time.

EDIT
Maybe using a while loop instead of a for loop would bring small speed increase, when using the second parameter the number of extis to check instead of the end exti.
for example: instead of

Code: Select all

dispatch_extis(5, 9); // start and end EXTI number
...
    /* Dispatch user handlers for pending EXTIs. */
    for (exti = start; exti <= stop; exti++) {
        uint32 eb = (1U << exti);
        if (pr & eb) {
            voidArgumentFuncPtr handler = exti_channels[exti].handler;
            if (handler) {
                handler(exti_channels[exti].arg);
                handled_msk |= eb;
            }
        }
    }

use

Code: Select all

dispatch_extis(5, 5); // start EXTI and number of EXTIs
...
    /* Dispatch user handlers for pending EXTIs. */
    while (nr_entis--) {
        uint32 eb = (1U << start);
        if (pr & eb) {
            handled_msk |= eb; // moved one line up
            exti_channels[start].handler(); // void, should be set to dummy if not used
        }
        start++;
    }

victor_pv
Posts: 1604
Joined: Mon Apr 27, 2015 12:12 pm

Re: Slow response to external ISR when using AttachInterrupt

Post by victor_pv » Mon Aug 28, 2017 3:04 pm

Steve,
I'm having a hard time understanding the improvement from this line:

Code: Select all

exti_channels[start].handler(); // void, should be set to dummy if not used
If a handler is not set, it would be Null, which would cause a crash if we jump to it, right?
So I guess that's why you say to set it do Dummy. We could set it to a function containing only return, but then for every line that has that dummy handler, it would still call that empty function that only returns, and that i believe could bring down the speed as the MCU would have to branch out to that function, then back again.
I would think that has a bigger speed penalty than checking a RAM value !=0, since it would not take the MCU out of that loop., and RAM access is at full speed.
I'm not understanding what speed improvement comes from not checking if the handler has a value other than NULL.

User avatar
RogerClark
Posts: 6692
Joined: Mon Apr 27, 2015 10:36 am
Location: Melbourne, Australia
Contact:

Re: Slow response to external ISR when using AttachInterrupt

Post by RogerClark » Mon Aug 28, 2017 10:31 pm

Victor

The handler function is never null, unless you call attach interrupt and pass null.

The change I was testing sets all handles to the dummy handler function, in the global init, and also in the detachInterrpt.

I agree that if the board is getting interrupts on pins which you don't expect, it will slow down a bit because of the call overhead to call the dummy handler, but even with the handler set to null and the extra if statement to check for that... it would slow the code down a hell of a lot. So I think we would have noticed if interrupts were enabled on pins that we had not expected them to be enabled upon.



Steve.

yes, if the argument that is passed to the handler is always null we may as well remove it

stevestrong
Posts: 1511
Joined: Mon Oct 19, 2015 12:06 am
Location: Munich, Germany

Re: Slow response to external ISR when using AttachInterrupt

Post by stevestrong » Tue Aug 29, 2017 8:36 am

victor_pv wrote:
Mon Aug 28, 2017 3:04 pm
We could set it to a function containing only return, but then for every line that has that dummy handler, it would still call that empty function that only returns, and that i believe could bring down the speed as the MCU would have to branch out to that function, then back again.
I would think that has a bigger speed penalty than checking a RAM value !=0, since it would not take the MCU out of that loop., and RAM access is at full speed.
You may be right with that.
So we can keep the check for NULL:

Code: Select all

/* Dispatch user handlers for pending EXTIs. */
while (nr_entis--) {
    uint32 eb = (1U << start);
    if (pr & eb) {
        handled_msk |= eb; // moved one line up
        register voidFuncPtr handler = exti_channels[exti].handler;
        if (handler) {
            handler();
        }
    }
    start++;
}
I think the removal of the arg will save a lot of run-time.

User avatar
RogerClark
Posts: 6692
Joined: Mon Apr 27, 2015 10:36 am
Location: Melbourne, Australia
Contact:

Re: Slow response to external ISR when using AttachInterrupt

Post by RogerClark » Tue Aug 29, 2017 10:35 am

But you can initialise the handlers to dummyHandler() here

Code: Select all

static exti_channel exti_channels[] = {
    { .handler = NULL, .arg = NULL },  // EXTI0
    { .handler = NULL, .arg = NULL },  // EXTI1
    { .handler = NULL, .arg = NULL },  // EXTI2
    { .handler = NULL, .arg = NULL },  // EXTI3
    { .handler = NULL, .arg = NULL },  // EXTI4
    { .handler = NULL, .arg = NULL },  // EXTI5
    { .handler = NULL, .arg = NULL },  // EXTI6
    { .handler = NULL, .arg = NULL },  // EXTI7
    { .handler = NULL, .arg = NULL },  // EXTI8
    { .handler = NULL, .arg = NULL },  // EXTI9
    { .handler = NULL, .arg = NULL },  // EXTI10
    { .handler = NULL, .arg = NULL },  // EXTI11
    { .handler = NULL, .arg = NULL },  // EXTI12
    { .handler = NULL, .arg = NULL },  // EXTI13
    { .handler = NULL, .arg = NULL },  // EXTI14
    { .handler = NULL, .arg = NULL },  // EXTI15
};

Code: Select all

static exti_channel exti_channels[] = {
    { .handler = dummyHandler, .arg = NULL },  // EXTI0
    { .handler = dummyHandler, .arg = NULL },  // EXTI1
    { .handler = dummyHandler, .arg = NULL },  // EXTI2
    { .handler = dummyHandler, .arg = NULL },  // EXTI3
    { .handler = dummyHandler, .arg = NULL },  // EXTI4
    { .handler = dummyHandler, .arg = NULL },  // EXTI5
    { .handler = dummyHandler, .arg = NULL },  // EXTI6
    { .handler = dummyHandler, .arg = NULL },  // EXTI7
    { .handler = dummyHandler, .arg = NULL },  // EXTI8
    { .handler = dummyHandler, .arg = NULL },  // EXTI9
    { .handler = dummyHandler, .arg = NULL },  // EXTI10
    { .handler = dummyHandler, .arg = NULL },  // EXTI11
    { .handler = dummyHandler, .arg = NULL },  // EXTI12
    { .handler = dummyHandler, .arg = NULL },  // EXTI13
    { .handler = dummyHandler, .arg = NULL },  // EXTI14
    { .handler = dummyHandler, .arg = NULL },  // EXTI15
};
and here

Code: Select all

void exti_detach_interrupt(exti_num num) {
    /* First, mask the interrupt request */
    bb_peri_set_bit(&EXTI_BASE->IMR, num, 0);

    /* Then, clear the trigger selection registers */
    bb_peri_set_bit(&EXTI_BASE->FTSR, num, 0);
    bb_peri_set_bit(&EXTI_BASE->RTSR, num, 0);

    /* Finally, unregister the user's handler */
    exti_channels[num].handler = dummyHandler;
    exti_channels[num].arg = NULL;
}
If someone passes null to the interrupt handler, its their own fault it crashes.

Most places if you try to use null pointers the code would crash

But to retain the same level of functionality it would be more effecient to check for a null pointer in

Code: Select all

void exti_attach_interrupt(exti_num num,
                           exti_cfg port,
                           voidFuncPtr handler,
                           exti_trigger_mode mode) {
    // Call callback version with arg being null
    exti_attach_callback(num, port, (voidArgumentFuncPtr)handler, NULL, mode);
}
And simply not call

Code: Select all

 exti_attach_callback();
Removal of the ARG is great idea, it saves time pushing vars onto the stack, and also memory as that array will need to be in RAM

stevestrong
Posts: 1511
Joined: Mon Oct 19, 2015 12:06 am
Location: Munich, Germany

Re: Slow response to external ISR when using AttachInterrupt

Post by stevestrong » Tue Aug 29, 2017 11:34 am

Roger, the issue is that to call the dummy function and to return from that takes more time than just checking the function pointer for NULL.
So it does not make too much sense to avoid the NULL pointer check by calling a dummy function (and to return from that).

I think the check the function pointer for NULL is safer when done within the ISR, to ensure that only valid functions will be executed. But your version (to check for NULL at registering time) is also feasible.

Anyway, I think we can agree on removing the ARG member from the array.

victor_pv
Posts: 1604
Joined: Mon Apr 27, 2015 12:12 pm

Re: Slow response to external ISR when using AttachInterrupt

Post by victor_pv » Tue Aug 29, 2017 3:54 pm

If the arg argument is not used, then yeah let's remove it. Any idea why was it there in the first place?
Perhaps leaflabs wanted to add some functionality and did not finish, or instead they were removing functionality and forgot to take that out?


About the branching to Dummy, I think Steve got my point, that in the F103 due to flash wait states jumping and returning to anything further than a couple of instructions ahead takes much longer than checking 1 RAM variable and then either continue execution with no penalty or do the jumping/returning.

EDIT: I see the arg is actually used. It is passed as parameter to the handler:
https://github.com/rogerclarkmelbourne/ ... xti.c#L284

From what I understand, it's intended so allow for a single user ISR to manage all ESXTI interrupt callbacks, so it gets the arg as a parameter when called back.
So I can register the same function to be called, but for line 1 the arg parameter could be a 1, and for line 2 arg could be a 2, so when my function is called I know which line is causing the call.
Of course I could write 2 different ISRs, but perhaps the function has to do exactly the same process, so writing twice the same function is a waste.

Post Reply

Who is online

Users browsing this forum: No registered users and 1 guest