Can't deploy C# to TinyCLR debug builds

[Nathan-Stohs]

Can't deploy C# to TinyCLR debug builds. Release builds work.

Broken in Master, offending commit seems to be 9936149e94c88ecb9f90d139dfbb711414fe267b.

[MichaelAtSamraksh]

I am paranoid that this has to do with GlobalLocks. I fixed the semantics for GlobalLocks one week ago. Same behavior: calling GlobalLock(var); creates a smart pointer named var and turns the interrupts off. New behavior: GlobalLocks nest. So as more code calls GlobalLock, the locks are nested on the stack, so interrupts are not re-enabled until the last global lock is released. Previously, while debugging some other issue, Nived fundamentally broke GlobalLocks to enable interrupts whenever any lock was destroyed (instead of the originating lock). I'm still working up profiling code to demonstrate the more subtle issues with that behavior (That's why I asked Nathan off-line about exposing the STM32F103's ARM ETM pins for instruction trace). I hope to document GlobalLocks on the wiki when the profiling results are complete. Bottom line: make sure you know where and when your GlobalLocks are releasing their locks. In the past, it was common for Nived to solve a problem by just adding a global lock at the top of some section of code, which led to the system slowing down to the point where Nived gave up and ripped out the GlobalLock nesting behavior. To recap, it's not wrong to use Global Locks, but it is bad to use Global Locks without thinking about what they're doing or where exactly they are needed. I have not reviewed Chris's commit that Nathan linked to, but I'm posting this because i'm paranoid that global locks are what caused the commit to create that behavior, and I want everyone to be aware of the new fixed behavior. If you no longer need a lock, you can call var.Release();. File is at https://github.com/Samraksh/MF/blob/master/MicroFrameworkPK_v4_3/DeviceCode/Targets/Native/STM32F10x/DeviceCode/cores/arm/Common/GlobalLock/gcc-thumb2/SmartPtr_thumb2.cpp

[Nathan-Stohs]

After talking to Chris on a related issue, I spent 20 minutes on this. I see two problems:

• Mike's lock fix exposed a big performance hit. He fixed a pretty big correctness issue but as a result we are only now seeing how expensive locks really are.
• The VT has performance issues or at least is pretty heavy.

If I ruthlessly remove locks from the MF PAL that are in the COM receive chain, I can eek out enough performance for TinyBooter/TinyCLR to respond to ping in debug mode at 48 MHz (high-power), but not 8 MHz (low-power).

If I then also replace the VT, I can further increase responsiveness enough for TinyBooter to be happy even down to 8 MHz in debug mode (something previously unheard of).

While it isn't absolutely critical that debug builds be fully capable, I would expect that this would have implications for user performance in the release builds as well.

• In the case of locking... I counted no less than 15 locks involved with the receipt of a single byte of junk. All but one of these locks were in the PAL layer (e.g. not our drivers). Somebody needs to walk through, do some profiling, and do some surgical (but likely small) re-writes.
• In the case of the VT, I'd again recommend profiling and a code performance pass. The VT is not really needed for TinyBooter so it can be removed wholesale there, but TinyCLR is what really matters. In particular, when debugging and I break randomly, I often see that I'm in the middle of an integer division in the VT code, so that is one place to start. EDIT: The above lock fix probably also impacted VT performance as well.
• Runner-up special mention is that COM port uses what is quite possibly the worst ever circular buffer C++ template implementation you could imagine. it's a double-whammy, because of our managed and unmanaged queues, at least for COM1, we have to hit this code twice.

What we are up against are the performance deadlines of the default baud rate (115200), our lack of hardware flow control, and the fragility of the underlying MF debug protocol. In the above mentioned cases, we just can't service the buffer fast enough and get RX overflows and the whole system falls apart.

Alternatively to directly improving performance, Mike informs me that we can recompile and distribute our own DLLs to slow the baud rate down to say 57600 or maybe even 19200. This is less desirable for a number of reasons, but maybe better than nothing.

[MichaelAtSamraksh]

This is larger than GlobalLocks. It also involves the NVIC priority system and pre-emption. Nived used full 16-level priority assignments, so higher priority IRQs could always interrupt lower priority IRQs. The interrupt controller's number of priority levels were modified (from the UART driver, tsk, tsk :0) when Nathan added the much needed UART TURBO mode, and it is not clear to me that anybody has a handle on balancing pre-emption with performance.

• [ ] Check the order of execution between Nathan's priority bit assignment in UART vs Nived's priority bit assignment in INTC (very easy), see what we're currently using.

I mentioned some of the lock usage may not be needed (this was the discussion about the radio).

• [ ] Identify all places where locks are used (easy)
• [ ] Identify places where locks could be used but are not (common sense guessing)
• [ ] Identify places where locks should be used but are held for too long (pretty much all IRQ handlers)
• [ ] Identify possible race conditions due to latency in the IRQ handlers (need profiling of long paths)
• [ ] Identify code paths that should never intertwine but are in different IRQs (i.e., different TIM1 IRQs)
• [ ] understand how the SystemStates variables are used in the CLR and whether they are needed.
• [ ] Identify whether we could use STM32 atomic transactions to update SystemStates at the entry to an IRQ instead of using GlobalLocks (medium effort, gets rid off many GlobalLocks in IRQ handlers if analysis says we need to keep the SystemStates variable.
• [ ] Identify where short cuts may be taken in VirtualTimers to improve performance.
• [ ] Identify hot spots in VirtualTimers (the functions that are often executed and should be inlined or rewritten)
• [ ] Check if .NET MF vNext has already received a COM port overhaul since virtual COM ports have been added to vNext.
• If we only distribute the Release flavor, we might not need to worry about customers deploying under debug mode.
• It would be super handy to have the compiler automatically generate a report about the longest call depth (like RVDS does). It seems that we're often all shocked at how long the code paths become.
• It would be super handy to have an ITM capability. The high-level profiling can be done on ADAPT but we're lacking in tools for the DotNow.
• The global locks sometimes represent extra function calls that don't need to happen.
• I support removing VT from TinyBooter since we need a solid, featureless, boot loader.
• I second the circular buffer needs re-work. It's needed re-work since before 2013.

[ChrisAtSamraksh]

This build stops a lot of needless banging on the virtual timer code and allows deployment in debug builds. 2e22b811a85ff3f68eeac437d6e948562dad3c1a Deployment does not seem as reliable as it has in the past so if we can improve code efficiency it can help.