bpo-32503: Avoid creating too small frames in pickles.#5127
Conversation
| frame_size = self.FRAME_SIZE_TARGET | ||
| num_frames = 20 | ||
| obj = [bytes([i]) * frame_size for i in range(num_frames)] | ||
| obj = {i: bytes([i]) * frame_size for i in range(num_frames)} |
There was a problem hiding this comment.
IIUC, small integers don't produce a frame? Can you add a comment?
There was a problem hiding this comment.
I may misunderstand, actually...
There was a problem hiding this comment.
I have added small objects (dict keys) between large objects.
| # to the pickle's end. | ||
| frame_size = len(pickled) - last_pos - last_frame_opcode_size | ||
| self.assertEqual(frame_size, last_arg) | ||
| self.assertLessEqual(pos, frame_end) |
There was a problem hiding this comment.
Good point. I tried to write these tests in different forms and some checks are left duplicated.
| if pos < frame_end: | ||
| self.assertNotIn(op.name, 'FRAME') | ||
| if op.name in frameless_opcodes: | ||
| self.assertLessEqual(len(arg), self.FRAME_SIZE_TARGET) |
There was a problem hiding this comment.
Could you add a comment explaining this?
| self.assertLessEqual(pos, frame_end) | ||
| self.assertLessEqual(pos, frame_end) | ||
| if pos < frame_end: | ||
| self.assertNotIn(op.name, 'FRAME') |
There was a problem hiding this comment.
This test looks strange. Perhaps you meant assertNotEqual(op.name, 'FRAME')?
| self.assertLessEqual(len(arg), self.FRAME_SIZE_TARGET) | ||
| else: | ||
| frame_end = None | ||
| #frameless_start = pos |
There was a problem hiding this comment.
Hmm, looks like this should be removed.
| (op.name in frameless_opcodes and | ||
| len(arg) > self.FRAME_SIZE_TARGET)): | ||
| if frameless_start is not None: | ||
| self.assertLess(pos - frameless_start, |
pitrou
left a comment
There was a problem hiding this comment.
Thanks for addressing the comments!
|
Thank you for your review! |
https://bugs.python.org/issue32503