Fix quadratic performance issue in list numbering

[kevinbackhouse]

This is the same fix as github#322. It fixes the bug described in GHSA-r8vr-c48j-fcc5.

To reproduce the bug:

python3 -c 'n = 10000; print("1.\n" + " 2.\n"*n)' | time ./src/cmark -t commonmark
python3 -c 'n = 10000; print("1.\n" + " 2.\n"*n)' | time ./src/cmark -t man

Increasing the number 10000 in the above command causes the running time to increase quadratically.

This PR replaces #472, which didn't work correctly (see github#321).

[jgm]

Thanks for submitting this! I haven't had a chance to look at it in detail yet.

[nwellnhof]

This modifies the source document for no good reason. Why not store the current list index in a local variable?

[kevinbackhouse]

This modifies the source document for no good reason. Why not store the current list index in a local variable?

Sorry, it's been a really long time since I wrote this code and I don't remember all the details. The performance issue is caused by this code:

cmark/src/commonmark.c

Lines 221 to 225 in 7d450d8

	tmp = node;
	while (tmp->prev) {
	tmp = tmp->prev;
	list_number += 1;
	}

It takes linear time to recompute the index, which is what causes the quadratic performance. I fixed it by caching the index on the node so that it doesn't need to be recomputed. As far as I can remember, I couldn't find a place to store it in a local variable, which is why I implemented it this way.

[jgm]

We don't want to modify the source, but if there's a way to handle this with a local variable, great.

In general I haven't been too worried about performance-related issues in the commonmark or man renderer. (HTML, yes.) And the issue is one that will only arise if you have an unusually long list.

[nwellnhof]

I just realized that for nested lists, you'd need a dynamic array to store list indexes. This would make things more complicated and adds a memory allocation to the rendering code which so far seems to be allocation-free. So I can see why the approach of storing indexes in the input document was chosen.

[jgm]

The rendering code isn't allocation-free; cmark_renderer has two dynamic string buffers, buffer and prefix, as well as a dynamic linked structure block_number_in_list_item that gets used in the man writer.

[kevinbackhouse]

Maybe a hashtable indexed by the node pointer? That would be functionally equivalent to storing data on the node itself, without actually modifying the node.

[jgm]

Something very similar to the existing block_number_in_list_item should do the trick. That is also a kind of stack, in order to handle nested lists. In addition to tracking the block number within the list item, we could track the list item item number. Perhaps the two features could even be integrated.