gh-118218: Reuse return tuple in itertools.pairwise

[hauntsaninja]

As per the comment, this is shamelessly based off of enumobject.c

With this change:

b1(1) min=152.8us mean=155.5us ± 3.8us (25 repeats, 1000 loops) b2(1) min=149.0us mean=159.1us ± 8.8us (25 repeats, 1000 loops) b3(1) min=232.6us mean=242.5us ± 11.7us (25 repeats, 1000 loops) b1(10) min=279.2us mean=296.9us ± 16.6us (25 repeats, 1000 loops) b2(10) min=249.5us mean=259.2us ± 12.2us (25 repeats, 1000 loops) b3(10) min=386.6us mean=398.8us ± 10.1us (25 repeats, 1000 loops) b1(1000) min=20.3ms mean=20.7ms ± 0.5ms (25 repeats, 1000 loops) b2(1000) min=16.7ms mean=17.1ms ± 0.2ms (25 repeats, 1000 loops) b3(1000) min=26.0ms mean=26.2ms ± 0.3ms (25 repeats, 1000 loops) 

Without this change:

b1(1) min=142.2us mean=143.0us ± 0.9us (25 repeats, 1000 loops) b2(1) min=142.7us mean=143.3us ± 1.0us (25 repeats, 1000 loops) b3(1) min=219.8us mean=227.2us ± 4.4us (25 repeats, 1000 loops) b1(10) min=314.2us mean=323.8us ± 4.1us (25 repeats, 1000 loops) b2(10) min=335.4us mean=341.8us ± 5.1us (25 repeats, 1000 loops) b3(10) min=362.0us mean=386.2us ± 14.9us (25 repeats, 1000 loops) b1(1000) min=26.5ms mean=27.3ms ± 0.3ms (25 repeats, 1000 loops) b2(1000) min=29.8ms mean=30.2ms ± 0.2ms (25 repeats, 1000 loops) b3(1000) min=26.0ms mean=26.5ms ± 0.4ms (25 repeats, 1000 loops) 

Benchmarking script:

Details

importstatisticsimporttimeimporttimeitfromitertoolsimportpairwisedefb1(n): forxinpairwise(range(n)): passdefb2(n): fora, binpairwise(range(n)): passdefb3(n): list(pairwise(range(n))) defformat_time(t): ift>=1e9: returnf'{t/1e9:.1f}s'ift>=1e6: returnf'{t/1e6:.1f}ms'ift>=1e3: returnf'{t/1e3:.1f}us'returnf'{t:.1f}ns'defformat_mean_stdev(ts): mean=statistics.mean(ts) stdev=statistics.stdev(ts) ifmean>=1e9: returnf'{mean/1e9:.1f}s ± {stdev/1e9:.1f}s'ifmean>=1e6: returnf'{mean/1e6:.1f}ms ± {stdev/1e6:.1f}ms'ifmean>=1e3: returnf'{mean/1e3:.1f}us ± {stdev/1e3:.1f}us'returnf'{mean:.1f}ns ± {stdev:.1f}ns'defbench(stmt): repeat=25number=1000timings=timeit.repeat( stmt, globals=globals(), repeat=repeat, number=number, timer=time.perf_counter_ns ) print( f'{stmt:16} 'f'min={format_time(min(timings))} 'f'mean={format_mean_stdev(timings)} 'f'({repeat} repeats, {number} loops)' ) bench('b1(1)') bench('b2(1)') bench('b3(1)') bench('b1(10)') bench('b2(10)') bench('b3(10)') bench('b1(1000)') bench('b2(1000)') bench('b3(1000)')

• Issue: Speed up itertools.pairwise #118218

[eendebakpt]

PyTuple_Pack is slow. I created an issue #118222 to discuss this.

An alternative that might be faster:

Suggested change

	po->result=PyTuple_Pack(2, Py_None, Py_None);
	PyObjecttmp[2] ={Py_None, Py_None};
	po->result=_PyTuple_FromArraySteal(tmp, 2),;

(note: untested!)

[eendebakpt]

I tested this locally. Since this line is once called once for each call to pairwise the performance difference is insignificant. It does matter for the other call to PyTuple_Pack (the one in pairwise_next), but that is best handled in a different PR.

[eendebakpt]

Since last_new is equal to old we could do:

Suggested change

	PyTuple_SET_ITEM(result, 0, Py_NewRef(old));
	PyTuple_SET_ITEM(result, 1, Py_NewRef(new));
	Py_DECREF(last_old);
	Py_DECREF(last_new);
	PyTuple_SET_ITEM(result, 0, old);
	PyTuple_SET_ITEM(result, 1, Py_NewRef(new));
	Py_DECREF(last_old);

[hauntsaninja]

It's not necessarily true that last_new is old. Really up to the whims of when the user lets go of the tuple

[rhettinger]

Conceptually this seems reasonable to me. Please do have someone else give it a thorough review (Serhiy would be a reasonable choice).

[serhiy-storchaka]

Please compare it also with the simple code that uses PyTuple_New + PyTuple_SET_ITEM instead of PyTuple_Pack if the latter is so slow.

[hauntsaninja]

PyTuple_New + PyTuple_SET_ITEM (maybe 1.05x faster than baseline):

b1(1000) min=27.0ms mean=27.3ms ± 0.1ms (50 repeats, 1000 loops) b2(1000) min=28.4ms mean=28.5ms ± 0.2ms (50 repeats, 1000 loops) b3(1000) min=25.2ms mean=25.7ms ± 0.3ms (50 repeats, 1000 loops) 

Using this diff:

Details

diff --git a/Modules/itertoolsmodule.c b/Modules/itertoolsmodule.c index 6ee447ef6a..668c5a4f15 100644 --- a/Modules/itertoolsmodule.c+++ b/Modules/itertoolsmodule.c@@ -356,7 +356,11 @@ pairwise_next(pairwiseobject *po) return NULL} /* Future optimization: Reuse the result tuple as we do in enumerate() */ - result = PyTuple_Pack(2, old, new);+ result = PyTuple_New(2);+ if (result != NULL){+ PyTuple_SET_ITEM(result, 0, Py_NewRef(old));+ PyTuple_SET_ITEM(result, 1, Py_NewRef(new));+ } Py_XSETREF(po->old, new); Py_DECREF(old); return result;

Baseline:

b1(1000) min=26.9ms mean=27.5ms ± 0.2ms (50 repeats, 1000 loops) b2(1000) min=29.5ms mean=29.7ms ± 0.3ms (50 repeats, 1000 loops) b3(1000) min=26.4ms mean=26.8ms ± 0.2ms (50 repeats, 1000 loops) 

This PR:

b1(1000) min=20.2ms mean=20.4ms ± 0.2ms (50 repeats, 1000 loops) b2(1000) min=16.9ms mean=17.0ms ± 0.0ms (50 repeats, 1000 loops) b3(1000) min=26.0ms mean=26.1ms ± 0.2ms (50 repeats, 1000 loops) 

[serhiy-storchaka]

LGTM.

[serhiy-storchaka]

In all tests above except for zip_longest there is a next(it) before gc.collect(). All these tests were added in 226a012 (bpo-42536).

@brandtbucher, why is such difference? Is there a bug in test_zip_longest_result_gc? Do we need next(it) there and here?

[serhiy-storchaka]

Thank you for your contribution @hauntsaninja.