gh-139476: Optimize range[:] slice

[StanFromIreland]

Benchmark script

import timeit

def bench(code, setup="r=range(1000)", n=500000):
    time = timeit.timeit(code, setup=setup, number=n)
    print(f"{code}: {time/n*1000000:.2f} us/op")

bench("r[:]")
bench("r[1:]")
bench("r[:-1]")
bench("r[1:10]")
bench("r[::2]")
bench("r[1:50:3]")

Quick little patch. Optimises the r[:] case nicely (~5x), with negligible impact on other cases (I assume it is just noise):

Case	Current (us)	PR (us)
r[:]	0.11	0.02
r[1:]	0.13	0.13
r[:-1]	0.18	0.19
r[1:10]	0.15	0.14
r[::2]	0.13	0.13
r[1:50:3]	0.16	0.16

I agree with Benedikt optimising this in general has little benefit, though I think one special case is acceptable. To cover all cases, it would require comparing the objects, resulting in a ~4x performance penalty and or some convoluted code.

• Issue: Apply similar slicing optimizations as tuple and str to range #139476

[picnixz]

I would instead have a look at how we optimize tuple slicing. strings and tuples check if the result slice would be equivalent to [:] and if so, return the object unchanged. For instance, we also want x[0:len(x)] to be x for strings for instance (this is achieved in unicode_subscript:

        } else if (start == 0 && step == 1 &&
                   slicelength == PyUnicode_GET_LENGTH(self)) {
            return unicode_result_unchanged(self);

If you were to make this change with this improved version, then we should also test non-trivial steps and decreasing sequences, as well as empty ones.

Otherwise, please add a comment saying that we only optimize r[:] but not r[0:len(r)].

[StanFromIreland]

How about the slightly convoluted, though covering several more cases:

    if ((slice->start == Py_None || (PyLong_Check(slice->start) && PyLong_AsLong(slice->start) == 0))
        && (slice->stop == Py_None || (PyLong_Check(slice->stop) && PyObject_RichCompareBool(slice->stop, r->length, Py_EQ) == 1))
        && (slice->step == Py_None || (PyLong_Check(slice->step) && PyLong_AsLong(slice->step) == 1)))
    {
        return Py_NewRef(r);
    }

Benchmarks

(Negligible differences for non-optimised cases, mean of one million runs)

current
r[:]: 0.12 us/op
r[0:]: 0.13 us/op
r[::]: 0.11 us/op
r[:len(r)]: 0.18 us/op
r[0:len(r):1]: 0.21 us/op
r[:len(r):10]: 0.19 us/op
r[1:]: 0.13 us/op
r[:-1]: 0.20 us/op
r[1:10]: 0.14 us/op
r[::2]: 0.12 us/op
r[1:50:3]: 0.15 us/op
r[0:50:3]: 0.16 us/op

pr
r[:]: 0.02 us/op
r[0:]: 0.03 us/op
r[::]: 0.02 us/op
r[:len(r)]: 0.08 us/op
r[0:len(r):1]: 0.09 us/op
r[:len(r):10]: 0.19 us/op
r[1:]: 0.13 us/op
r[:-1]: 0.19 us/op
r[1:10]: 0.14 us/op
r[::2]: 0.12 us/op
r[1:50:3]: 0.15 us/op
r[0:50:3]: 0.17 us/op

[picnixz]

Why complicate this. Can't you use _PySlice_GetLongIndices to get the values and then use them directly?

[StanFromIreland]

Same situation with the benchmarks, albeit slightly slower now for the special cases, down to ~3x faster.

[picnixz]

I'm still unsure whether this is really needed.

[picnixz]

PyObject_RichCompareBool can raise an error. Handling it would then make the code more complex. So I'm rather against that change as it's not so trivial.

Also, start == _PyLong_GetZero() is not correct because it's not guaranteed that 0 will always be immortal (though it should in general and I see that there is code that is doing this for 1). I actually thought that we had the indices as C integers and not Python ones, but it appears that it's not the case.

[StanFromIreland]

I'm still unsure whether this is really needed.

I have the same sentiment, I think the two best options are closing this or just implementing the initial small patch.

[picnixz]

I will close this one as not planned for now. If there is a real reason for optimizing this path, we'll revisit later but the issue rationale doesn't seem to be backed by any practical considerations.