✅ Deploy Preview forpython-cpython-preview ready!

Name	Link
🔨 Latest commit	fdc644d
🔍 Latest deploy log	https://app.netlify.com/sites/python-cpython-preview/deploys/639242735133a90008643bb8
😎 Deploy Preview	https://deploy-preview-100118--python-cpython-preview.netlify.app/library/itertools
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to yourNetlify site settings.

ThePy_SET_SIZE(result, i) is more delicate for tuples, sincethey usePy_SIZE() to determine which freelist they get put onto.

Checkingi == 0 is the only thing preventing this from crashing atassert(!PyTuple_CheckExact(op)); intupledealloc. If I'm understanding right, a tuple allocated for millions of items could linger around on the freelist of 2-tuples.

I think it might be safer to call_PyTuple_Resize in the underfilled case. That's whatthetuple() constructor does as well.

I thought about as well, but what we're doing is reasonable. It just leaves the tuple over-allocated which does not defeat any subsequent reuse. ThePy_SET_SIZE is faster and simpler than_PyTuple_Resize() does (which includes possibly building a whole new tuple). The tuple constructor has different concerns — there it is more important not to waste space. Here, we really don't care even in the rare case where that batch size is large.

Hmph, I pushed the green button without giving you a chance to respond. Sorry about that. If you feel strongly about_PyTuple_Resize(), then we can continue the conversation and I'll look at it again. I don't see any way to view this as "unsafe"; there's just a question about whether it is undesirable for an overallocated tuple to be put in the reuse pool. Since `Tuple_MAXSAVESIZE' is appropriately set to a small size (currently 20), it doesn't matter.

No worries. I agree that there is no genuine "safety" concern, just a question of memory overuse here and there.

This was my fear: someone may be doing an operation that involves very high per-batch overhead, and so they choose a large batch size, say 0.1% of working memory. I've written code roughly like this before, and probably would have usedbatched() had it existed:

BATCHSIZE=1_000_000defget_data(db_cursor,ids):forbatchinbatched(ids,BATCHSIZE):db_cursor.executemany(SQL_GET_DATA,batch)yieldfromdb_cursor.fetchall()deffile_pairs():    ...forin_file,out_fileinfile_pairs():ids=get_data_from_file(in_file)forrowinget_data(db_cursor,ids):append_data_to_file(out_file,row)

If there are many files withlen(ids) % BATCHSIZE < 20, includinglen(ids) < 20, then there could be up to around2_000*20*BATCHSIZE extra words caught up in various tuples.

Some mitigating factors though:

• I imagineBATCHSIZE will often be hand-tuned and hard-coded, so if someone notices too much memory usage, they can shrink the batchsize.
• Freelists will often be full, so the stored tuples may not be added to the freelist anyway. Only specific allocation patterns matter.

But this data-dependence leads to another issue: the code seems like it ought to take a large constant amount of memory, but for some obscure inputs (many small leftover batches), it could take 40000x as much memory, caught up in mysterious places like thefile_pairs() tuples or database rows. Even if every file happens to have exactly1_000_007 ids, we wind up with 2000*1000000 allocated words in the length-7 freelist.

I don't really know how to assess how probable these are, especially with the LIFO nature of our freelists, so my thought was to do the more predictable thing and be as conservative as possible with memory usage, giving things back to the operating system as soon as possible.

But maybe my usage of huge batches is playing with fire anyway, so it would be understandable to keep the code as is for that reason.

Another option would be to only do a_PyTuple_Resize if the result will have a length that belongs in a freelist, though I suppose that makes the code uglier. Or do aslist_resize does and only realloc if we're under half full.

Thoughts?

The scenario seems bogus, but I can't divert any more time to this. So, I'veput the resize in.

I quite liked thatPy_SET_SIZE compiled to a single instruction,str x23, [x19, #16]. The new code path is 100x larger and will partially blow-out the instruction cache. I have mixed feelings about this. On the one hand, it assuages worries and might prevent some future issue from arising. On the other hand, it falls in the category of "make everyone pay for a problem that almost no one has". The notion of an overallocated tuple floating around seems to be too much to bear.