xoofx / xoofx/markdig

Some optimization ideas on .NET Standard/Core w/ System.Memory

Open
#413 16 comments 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

enhancement
Dominant language
C#
Stars
5.3k
Forks
510
Avg merge
8d 5h
Merged PRs (30d)
5

Description

Hi, I discovered this library the other day during a Twitch stream, and I want to say amazing work with this project, it looks great! I spent a bit poking around the codebase and I really appreciate how well organized it is, and lots of small tricks you're doing all over the place (eg. BitVector128 comes to mind), so props to all the mainteiners for the great work! 👏

I'm opening this issue as I was wondering whether you'd consider taking a dependency on System.Span on .NET Standard 2.0 (and .NET Core 2.1 too, why not) to enable additional optimizations. Specifically, I noticed a lot of helper methods in the CharHelper class contain a fair bit of branching, which could be removed almost entirely by leveraging the C# 7.3 optimization with ReadOnlySpan<T> instances wrapping constant arrays being compiled to constant data in the .text segment.

I'm sure there could be other optimizations that could be done as well once having access to Span<T> and the Unsafe APIs, this is just the first that came to mind. Here's an example, consider this method:

https://github.com/lunet-io/markdig/blob/abc8aa25b72f1546ae625048bc35e6392fbba143/src/Markdig/Helpers/CharHelper.cs#L322-L362

On .NET Core 3.1 x64, it's JITted to this assembly:

C.IsAsciiPunctuation_Slow(Char)
    L0000: movzx eax, cx
    L0003: lea edx, [rax-0x21]
    L0006: cmp edx, 0x1f
    L0009: ja L0023
    L000b: mov eax, edx
    L000d: lea rdx, [rip+0x2c]
    L0014: mov edx, [rdx+rax*4]
    L0017: lea rcx, [rip-0x1e]
    L001e: add rdx, rcx
    L0021: jmp rdx
    L0023: lea edx, [rax-0x5b]
    L0026: cmp edx, 0x5
    L0029: jbe L0033
    L002b: add eax, 0xffffff85
    L002e: cmp eax, 0x3
    L0031: ja L0039
    L0033: mov eax, 0x1
    L0038: ret
    L0039: xor eax, eax
    L003b: ret

The switch in C# is converted into just 3 jump tables in IL, so the final assembly only contains 3 jumps, but still. As a proof of concept, here's the same method using a lookup table:

private static ReadOnlySpan<byte> AsciiPuntuationMap => new byte[]
{
    0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
    0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
    0, 1, 1, 1, 1, 1, 0, 1, 1, 1, 1, 1, 1, 1, 1, 1,
    0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 1, 1, 1, 1,
    1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
    0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 1, 1, 1,
    1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
    0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 1, 1, 0
};

internal static bool IsAsciiPunctuation(this char c)
{
    bool isInRange = c <= 127;
    byte rangeFlag = Unsafe.As<bool, byte>(ref isInRange);
    int
        negativeFlag = rangeFlag - 1,
        mask = ~negativeFlag,
        offset = c & mask;
    ref byte r0 = ref MemoryMarshal.GetReference(AsciiPuntuationMap);
    byte r1 = Unsafe.Add(ref r0, offset);
    bool found = Unsafe.As<byte, bool>(ref r1);

    return found;
}

Which again on .NET Core 3.1 x64 is JITted to this assembly:

C.IsAsciiPunctuation(Char)
    L0000: movzx eax, cx
    L0003: cmp eax, 0x7f
    L0006: setle dl
    L0009: movzx edx, dl
    L000c: dec edx
    L000e: not edx
    L0010: and eax, edx
    L0012: movsxd rax, eax
    L0015: mov rdx, 0x1ff23aa0b20
    L001f: movzx eax, byte [rax+rdx]
    L0023: ret

Shorter code overall, and no conditional branches at all 😄

The same exact approach could be replicated for many other similar helpers, just with a separate lookup table. Here is a sharplab.io repro with both versions of that method, for comparison.

If it helps, I'm also integrating that optimized logic to index/clamp the lookup table with an index into an extension in the HighPerformance package in the Microsoft.Toolkit library, so one option could also be to take a dependency on that package instead once it's out, so that you wouldn't even have to add that code in here, just the lookup table and two lines to access it, which would help maintainability.

Just thought I'd share this idea, congrats again for the great work here!
Cheers! 🍻

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with src/Markdig/Helpers/CharHelper.cs, especially the IsAsciiPunctuation method linked in the issue, and review the .NET Standard/Core dependency options for System.Span and Unsafe. The issue proposes lookup-table replacements for similar helpers, but the intended scope and preferred dependency are not decided; done would require an agreed optimization plan and corresponding validation.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
performance
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.