fix(perf): Remove FontSources.masks as they were consuming large amounts of memory even when no font sources were set

[Auspicus]

fixes #2518

This drops the RSS value after the bitsets are computed. Unfortunately the bitset computation still spikes the memory usage quite high.

UPDATE: Swapping out BitSet for BTreeSet results in much lower peak memory usage after startup. I don't think BitSet is doing what we want here.

UPDATE: Replaced BTreeSet back with BitSet for the font codepoints for lower peak memory usage for a font that provides every available codepoint.

[CommanderStorm]

@nyurik Why did you use masks in the first place here instead of something like

Suggested change

	let needed: &BTreeSet<usize> = &self.masks[(start as usize) / CP_RANGE_SIZE];
	// Create a BitSet for the requested range (start..=end)
	let mut needed = BitSet::with_capacity(end as usize);
	for cp in start..=end {
	needed.insert(cp as usize);
	}

In the testing I have done, this works fairly well, but there must be a reason why this is done this way that I am not getting..

[nyurik]

tbh, it has been a long time since i did it, so no clue :)

[Auspicus]

I can imagine BitSet would work well for a small range like 0-255 but if you support the whole unicode range you need a lot of memory per bitset.

[CommanderStorm]

Unsure if there is a better way.
Lets brainstorm a bit.

so we need to know which if the 256 bits -> BitSet256 is set.

While we curently store this as one very big BitSet, that is likely not nessesary, since this range is likely fairly compressible.
No font designer would leave weird gaps in their font intentionally I think.

• Vec<BitSet256> works and weighs in at ~1.1 megabytes (MAX_UNICODE_CP Bits)
• BTreeSet<u32> works and is 32B * number of font-items -> Likely lower than above.
• HashMap<u16, BitSet256> ((start / 256) -> next 256 code points) should works and is a compromise between the two?

[CommanderStorm]

Noticed that only u32 is needed for the index, that is another halfing of that memory usage.

This also stacks with the improvement above.

[Auspicus]

Yeah there was a mix of usize and u32 around so definitely could standardise on u32 and avoid a bunch of casting

[Auspicus]

The problem with using a BitSet256 is you need to range it from 0-255 but the codepoints are 0-0x10FFFF so you either need to translate the requested codepoints back to 0-255 before masking (losing the performance of a bitset) or have one GIANT BitSet for each range. If you check the size of each BitSet before you'll see some are as big as 0x10FFFF (depending on the largest inserted value).

[Auspicus]

I think the core issue is that although we allocate a BitSet::with_capacity(256) the value for v here might be 0x10FFFF causing a MASSIVE reallocation to fit values 0-0x10FFFF in the BitSet. From what I understand, the values must be contiguous without holes so each BitSet could be as large as (0x10FFFF / 32) (bits per u32) * 4 (bytes per u32) and then each of these is inserted for each range up to 4352.

[nyurik]

my understanding is that each file would use a 136KB of RAM (136*1024*8 = 1_114_112 = 0x11_0000 codepoint presence bits) - not that much memory. If we follow the optimization I proposed below, most fonts won't even get that high, but it will keep the complexity to the minimal.

[Auspicus]

The issue is less with each font source since it's only a single BitSet of 34816 u32 (139KB). The issue is with the pre-computed masks. Each range (of which there are 4352) carries a masking BitSet of up to 0x11_0000 (34816 u32 [4 bytes] in the backing BitVec). Assuming every range is this size (inaccurately) we get 600MB (34816*4*4352 bytes). More accurately we get masking BitSets with sizes growing from 8-34816 u32 and more likely around 300MB total. Prior to adjusting the max codepoint size we had smaller BitSets (8-2048 u32) and less of them 256 (0xFFFF/256). Resulting in at most ~2MB (2048*4*256 bytes) for masks but likely less due to not all BitSets being 2048 size.

[Auspicus]

See images attached with debugger running on main branch in the default for FontSources:

[nyurik]

one thing we could try without any other changes:

 fn get_available_codepoints(face: &mut Face) -> Option<GetGlyphInfo> {
-    let mut codepoints = BitSet::with_capacity(MAX_UNICODE_CP);
+    let mut codepoints = BitSet::new();  // dynamic memory growth
 ...
     for cp in 0..=MAX_UNICODE_CP {
     ...
+      if count >= face.num_glyphs() { break; } // we know we won't find any more glyphs in this font

[nyurik]

P.S. Oh, silly me -- we no longer need to even do that -- I added character iteration to the freetype lib 2 years ago -- PistonDevelopers/freetype-rs#257

so should just be able to call for ch in face.chars() { ... }

[Auspicus]

So maybe no masks? I was wondering how expensive it would be to iterate vs difference the set in the first place

[nyurik]

freetype-rs doesn't support iteration from arbitrary location (PRs welcome i guess), so i think it might still be useful to have a bitmask for perf reasons, esp since it is much simpler now. Since you ran all the memory usage tests, could you try this implementation?

fn get_available_codepoints(face: &mut Face) -> Option<GetGlyphInfo> {
    let mut codepoints = BitSet::new();
    let mut spans = Vec::new();
    let mut first: Option<usize> = None;
    let mut last = 0;

    for (cp, _) in face.chars() {
        codepoints.insert(cp);
        if let Some(start) = first {
            if cp != last + 1 {
                spans.push((start, last));
                first = Some(cp);
            }
        } else {
            first = Some(cp);
        }
        last = cp;
    }

    if let Some(first) = first {
        spans.push((first, last));
        let start = spans[0].0;
        let end = spans[spans.len() - 1].1;
        Some((codepoints, usize::try_from(face.num_glyphs()).unwrap(), spans, start, end))
    } else {
        None
    }
}

[nyurik]

(didn't test, but had to update the above a few times)

[nyurik]

interestingly enough, my original implementation seemed to have had a bug - if a font file had a codepoint at the MAX_UNICODE_CP-1 position (last allowed), the last span would not be included in the result.

[nyurik]

(note to self - github editor is NOT that great for ... editing code)

@Auspicus have you had a chance to memory profile it?

[Auspicus]

Should fix this number which is now 0x10FFFF/256 = 4352 (rounded up)

[Auspicus]

The issue is less with each font source since it's only a single BitSet of 34816 u32 (139KB). The issue is with the pre-computed masks. Each range (of which there are 4352) carries a masking BitSet of up to 0x11_0000 (34816 u32 [4 bytes] in the backing BitVec). Assuming every range is this size (inaccurately) we get 600MB (34816*4*4352 bytes). More accurately we get masking BitSets with sizes growing from 8-34816 u32 and more likely around 300MB total. Prior to adjusting the max codepoint size we had smaller BitSets (8-2048 u32) and less of them 256 (0xFFFF/256). Resulting in at most ~2MB (2048*4*256 bytes) for masks but likely less due to not all BitSets being 2048 size.

[Auspicus]

See images attached with debugger running on main branch in the default for FontSources:

[Auspicus]

I am just starting my workday now but I'll review again after work today. I will also post the dhat file in the viewer as it really highlights the BitSet allocations.

[Auspicus]

Without explicit capacity there will be some required resizing but it allows for this BitSet to be as small as 8 bytes (256 bits) when the font only provides codepoints within the range 0-255. If the font provides even a single codepoint at the end of the range this will need to be resized to 139KB. So we're trading off a small amount of start up time here for a lower memory footprint per font.

[Auspicus]

Before:

git checkout main && \
cargo b --release --no-default-features --features fonts && \
valgrind --tool=dhat ./target/release/martin -C 0 -W 16 --font ./tests/fixtures/fonts

Total:     5,770,215,231 bytes in 90,450 blocks
At t-gmax: 4,857,098,381 bytes in 71,914 blocks
At t-end:  4,857,098,381 bytes in 71,914 blocks
Reads:     5,241,483,644 bytes
Writes:    5,475,459,168 bytes

After:

git checkout fix/2518/fonts-memory-usage && \
cargo b --release --no-default-features --features fonts && \
valgrind --tool=dhat ./target/release/martin -C 0 -W 16 --font ./tests/fixtures/fonts

Total:     8,947,164 bytes in 16,304 blocks
At t-gmax: 1,674,288 bytes in 4,653 blocks
At t-end:  96,933 bytes in 191 blocks
Reads:     4,826,227 bytes
Writes:    8,527,250 bytes

[Auspicus]

I'm happy with where this is at now, let me know if you think we need to change anything @CommanderStorm / @nyurik

[CommanderStorm]

Thank you 👍

[nyurik]

oops, we should have added to the subject / changelog that this also fixes incorrect font behavior - now the font glyph count will be reported correctly