gh-107369: optimize textwrap.indent()

[methane]

indent()-ing Object/unicodeobject.c (15332 lines) about 25% faster.

• Issue: Optimize textwrap.indent() #107369

[eendebakpt]

Looks good! Using str.split for the predicate instead of line.strip might change something for input that is not str, but I think this is ok.

[serhiy-storchaka]

lstrip is faster for non-indented lines.

I wonder whether the following variants can be faster for some input and for how wide category of input.

def predicate(line):
    return line and (not line[0].isspace() or line.lstrip())

or

predicate = re.compile(r'\S').search

[methane]

• _has_nonspace = re.compile(r'\S').search in global and predicate = _has_nonspace -- 3.5ms
• str.rstrip = 1.95ms
• str.lstrip = 2.03ms
• lambda x: not x.isspace() = 2.07ms

Since we use splitlines(keepends=True), we can use just not x.isspace(). (no empty line is guaranteed. "".splitlines(keepends=True) == [] and "foo\n".splitlines(True) == ['foo\n']).
But it is a bit tricky and has relatively high cognitive load.

In case of unicodeobject.c, rstrip is bit faster. But it may be because most lines are indented already.

So I chose str.lstrip here, as Serhiy suggested.

[serhiy-storchaka]

Now that you mention it, I can see that using isspace() is the most obvious way to do this. Why I did not see it earlier?

We want to test whether the line has any non-space character. bool(line.strip()) is actually a tricky way -- we strips the line from spaces and if the rest is not empty string, then the original line has non-space characters too. not line.isspace() is a straightforward way -- it asks the opposite question (is the line only contains space characters?) and negates the result.

Algorithmically, isspace() looks more preferable, because it does not create a string. But on practice it may not matter in common cases. Did you compare variants with different inputs? For example Misc/NEWS.d/3.8.0a1.rst may show a very different result.

[methane]

Now that you mention it, I can see that using isspace() is the most obvious way to do this. Why I did not see it earlier?

Because "".isspace() is False. We need to guarantee that "" is not used here.
x and not x.isspace() would be bit obvious, but little slower.

Algorithmically, isspace() looks more preferable, because it does not create a string. But on practice it may not matter in common cases. Did you compare variants with different inputs? For example Misc/NEWS.d/3.8.0a1.rst may show a very different result.

lstrip() is slow when every line has long indent. But Misc/NEWS.d/3.8.0a1.rst has almost no indents.

With 4c6a46a and https://gist.github.com/methane/5c6153c564d9508199a81c48d33161eb

> ./python.exe bench_indent.py Misc/NEWS.d/3.8.0a1.rst
filename='Misc/NEWS.d/3.8.0a1.rst' 8978 lines.
                   lstrip: 0.736msec
          not x.isspace(): 0.877msec
    x and not x.isspace(): 0.929msec

> ./python.exe bench_indent.py Objects/unicodeobject.c
filename='Objects/unicodeobject.c' 15332 lines.
                   lstrip: 1.812msec
          not x.isspace(): 1.877msec
    x and not x.isspace(): 1.970msec

If I add text = textwrap.indent(text, " "*32) before bench:

> ./python.exe bench_indent.py Objects/unicodeobject.c
filename='Objects/unicodeobject.c' 15332 lines.
                   lstrip: 2.259msec
          not x.isspace(): 2.356msec
    x and not x.isspace(): 2.437msec

[methane]

To maximize performance, we can stop using lambda by...:

    if predicate is None:
        for line in text.splitlines(True):
            if not line.isspace():
                prefixed_lines.append(prefix)
            prefixed_lines.append(line)
    else:
        for line in text.splitlines(True):
            if predicate(line):
                prefixed_lines.append(prefix)
            prefixed_lines.append(line)

filename='Objects/unicodeobject.c' 15332 lines.
                     None: 1.604msec
                   lstrip: 1.826msec
          not x.isspace(): 1.883msec

[serhiy-storchaka]

Thank you for your research Inada-san. Which to use here, lstrip or isspace, I leave up to you. It does not really matter in most cases.

[picnixz]

For very long texts, I think changing

prefixed_lines = []
for line in text.splitlines(True):
    if not line.isspace():
        prefixed_lines.append(prefix)
    prefixed_lines.append(line)

into the following may improve the overall performances

prefixed_lines = []
append_line = prefixed_lines.append
for line in text.splitlines(True):
    if not line.isspace():
        append_line(prefix)
    append_line(line)

EDIT: After a more careful benchmarking, this does not seem to bring more improvements. However, not using a lambda function seems to be better.