Issue1192789
This issue tracker has been migrated to GitHub,
and is currently read-only.
For more information,
see the GitHub FAQs in the Python's Developer Guide.
Created on 2005-04-29 23:46 by jamesh, last changed 2022-04-11 14:56 by admin. This issue is now closed.
Files | ||||
---|---|---|---|---|
File name | Uploaded | Description | Edit | |
python-2.4.1-visibility.patch | jamesh, 2005-04-29 23:46 | python-2.4.1-visibility.patch |
Messages (6) | |||
---|---|---|---|
msg48281 - (view) | Author: James Henstridge (jamesh) | Date: 2005-04-29 23:46 | |
GCC4 includes some new features that make it easier to control what symbols are exported from a library for linking. The attached patch makes Python use these features, limiting the exported symbols to the public Python C API (i.e. the same symbols as are exported under Windows). The patch sets the ELF visibility of the public Python APIs to "protected", which means that the symbol is exported but internal calls to those symbols are done as direct function calls. In my local tests on i386 Linux, pystone and parrotbench run about 5% faster on a "--enable-shared" build of Python with the patch applied. For a non shared library build of Python, performance is pretty much identical (I'll probably need to do some more tests). However it still has the benefit of not exporting private Python API. The patch also touches the init routines of a number of extension modules, since they weren't marked with PyMODINIT_FUNC, which caused them to build incorrectly with the patch (the init function ended up being private, so Python couldn't load the module). I've only tested this patch against 2.4.1, but it probably applies without too much trouble to the 2.5 development line. |
|||
msg48282 - (view) | Author: James Henstridge (jamesh) | Date: 2005-05-10 02:47 | |
Logged In: YES user_id=146903 I just tried out the patch on an AMD64 Linux machine. There is some improvement in the --enable-shared case, but it doesn't seem to be as big as on i386. I only tested pystone though, since parrotbench gives an assertion error on this box. |
|||
msg48283 - (view) | Author: Martin v. Löwis (loewis) * | Date: 2005-08-07 21:49 | |
Logged In: YES user_id=21627 Doesn't this cause backwards compatibility problems with extension modules which fail to declare their entry point with PyMODINIT_FUNC? Such modules would stop working when recompiled with the patch, right? Also, shouldn't the -fvisibility argument only be used if the __attribute__ syntax is supported? |
|||
msg48284 - (view) | Author: James Henstridge (jamesh) | Date: 2005-08-15 11:07 | |
Logged In: YES user_id=146903 The PyMODINIT_FUNC() problem could be fixed by filtering out the -fvisibility=hidden flag from Makefile.pre.in (this would require some more extensive build changes). You are right about the patch relying on -fvisibility only being implemented by compilers that implement the visibility attribute. I think this is currently true, but it would be better not to rely on it. In the time since I made the patch though, I have been told about a problem with the "protected" visibility modifier: on Linux systems at least, it slows down the symbol lookup process. Since code inside the same library as the protected symbol will be using a direct reference and code outside will be going through the PLT, the dynamic linker needs special code to return different addresses for the symbol depending on what code is calling it (this is slower than the code path for "default" visibility symbols). This is required in order to satisfy the C standard's requirements that comparisons of pointers to functions be simple and fast. The way around this is to use two functions: one is a "hidden" visibility function that is used by the code inside libpython, and the other is an alias of the hidden function with normal visibility and the function's advertised name. Unfortunately, this is more invasive to implement (but not overly so -- libxml2 and GTK contain such code). |
|||
msg48285 - (view) | Author: Martin v. Löwis (loewis) * | Date: 2005-08-15 20:38 | |
Logged In: YES user_id=21627 So would you agree that the patch should be rejected, then? I would classify it as "nice to have" - it doesn't fix an actual bug, nor does it add an important new feature. |
|||
msg48286 - (view) | Author: Martin v. Löwis (loewis) * | Date: 2006-04-15 08:45 | |
Logged In: YES user_id=21627 Rejecting the patch. If you ever find a solution to the issues discussed, please submit a new patch. |
History | |||
---|---|---|---|
Date | User | Action | Args |
2022-04-11 14:56:11 | admin | set | github: 41927 |
2005-04-29 23:46:58 | jamesh | create |