PyMarshal_ReadLastObjectFromFile

[paulfelix]

BPO	770280
Nosy	@terryjreedy, @mdickinson, @pitrou, @tiran
Superseder	bpo-7332: python script segment fault at PyMarshal_ReadLastObjectFromFile in import_submodule
Files	marshal.c.patch: The patch file marshal.c.patch2: The patch file (alternative approach)

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2010-04-29.17:50:49.996>
created_at = <Date 2003-07-12.20:42:08.000>
labels = ['interpreter-core']
title = 'PyMarshal_ReadLastObjectFromFile'
updated_at = <Date 2010-04-29.17:50:49.992>
user = 'https://bugs.python.org/paulfelix'

bugs.python.org fields:

activity = <Date 2010-04-29.17:50:49.992>
actor = 'mark.dickinson'
assignee = 'none'
closed = True
closed_date = <Date 2010-04-29.17:50:49.996>
closer = 'mark.dickinson'
components = ['Interpreter Core']
creation = <Date 2003-07-12.20:42:08.000>
creator = 'paulfelix'
dependencies = []
files = ['5463', '5464']
hgrepos = []
issue_num = 770280
keywords = ['patch']
message_count = 6.0
messages = ['44265', '44266', '59188', '59249', '104543', '104545']
nosy_count = 5.0
nosy_names = ['terry.reedy', 'mark.dickinson', 'pitrou', 'christian.heimes', 'paulfelix']
pr_nums = []
priority = 'normal'
resolution = 'out of date'
stage = None
status = 'closed'
superseder = '7332'
type = None
url = 'https://bugs.python.org/issue770280'
versions = ['Python 2.6']

[paulfelix]

This is a new implementation of the performance boost
in PyMarshal_ReadLastObjectFromFile where the pyc file
data is loaded into memory before parsing.

The original logic is as follows:

• Get the file size.
• If the file size is 16K or less, allocate a 16k char
  buffer on the call stack and load the file data into
  the buffer.
• Otherwise, if file size is 256k or less, malloc a
  temp char buffer from the heap and load the file data
  into the buffer.
• Otherwise, don't use a buffer.

Allocating 16k on the call stack can cause a stack
overflow when doing nested imports in a secondary
thread. The default thread stack size on some systems
is quite small (64k on HP-UX!). Also,
allocating/freeing memory on the heap for this purpose
may not be warranted.

This patch proposes a new implentation that allocates a
static 256k char buffer and uses a thread lock to keep
it thread-safe. The lock/unlock function logic was
taken from import.c.

Note: performance tests show an improvment on some
platforms when the static char buffer is defined at the
file level instead of inside the function.

[paulfelix]

Logged In: YES
user_id=819103

Adding 256k to the core footprint is probably not acceptable
either. How about 16k? Patch 2 is a compromise that uses the
original logic, but allocates the 16k buffer from static memory.

[tiran]

Is the patch still interesting for Python 2.6?

[paulfelix]

Maybe not. With the addition of thread.stack_size([size]), it's fairly
easy to get around the problem by bumping up the stack size. It would be
nice if PyMarshal_ReadLastObjectFromFile didn't allocate a small file
buffer on the stack (think of the impact when *recursively* importing
many .pyc files).

[terryjreedy]

Can this be closed as either out-of-date or rejected?

[mdickinson]

I think this is superseded by bpo-7332, which is now fixed in trunk (but not in 2.6).