homepage

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.

classification
Title: Implement PyOS_CheckStack on macOS using pthread_get_stack*_np
Type: enhancement Stage: patch review
Components: Interpreter Core Versions: Python 3.8
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: ZackerySpytz, corona10, ronaldoussoren, vstinner
Priority: low Keywords: patch

Created on 2018年06月25日 11:18 by ronaldoussoren, last changed 2022年04月11日 14:59 by admin.

Pull Requests
URL Status Linked Edit
PR 8046 closed corona10, 2018年07月02日 09:54
Messages (15)
msg320415 - (view) Author: Ronald Oussoren (ronaldoussoren) * (Python committer) Date: 2018年06月25日 11:18
The (non-portable) pthread APIs "pthread_get_stacksize_np()" and "pthread_get_stackaddr_np()" can be used to implement PyOS_CheckStack on macOS, which would give nicer behavior than crashing when the recursion limit is too high for the stack size of the current thread.
Creating a patch should be fairly easy, but does require C knowledge.
msg320786 - (view) Author: Dong-hee Na (corona10) * (Python committer) Date: 2018年06月30日 15:21
@ronaldoussoren
Hi, Ronald
Can I take a look this issue?
msg320842 - (view) Author: Ronald Oussoren (ronaldoussoren) * (Python committer) Date: 2018年07月01日 18:57
Sure. I'm not working on this at the moment.
msg320868 - (view) Author: Dong-hee Na (corona10) * (Python committer) Date: 2018年07月02日 10:47
@ronaldoussoren
I wrote a first proto type PR and it works well on my MacBook Pro.
As you commented, there is a issue with pthread_get_stacksize_np when pthread_main_np() == 1 on some old mac system.
I will take a look more deeply at this issue.
If you have any feedback for my first PR please left it. 
Thanks,
Dong-hee
msg320874 - (view) Author: Ronald Oussoren (ronaldoussoren) * (Python committer) Date: 2018年07月02日 12:08
Have you looked at benchmark results for this patch?
In particular, PyOS_CheckStack is called quite often and I wonder how those calls affect performance. If there is a clear performance impact it would be useful to store some information in the python thread state.
In what versions of macOS are there problems with pthread_get_stacksize_np() on the main thread?
msg320879 - (view) Author: Dong-hee Na (corona10) * (Python committer) Date: 2018年07月02日 13:04
@ronaldoussoren
1. I will try to run a benchmark is there any good benchmark to try it?
2. As far as I know, OS X 10.9 Mavericks has an issue with pthread_get_stacksize_np, I added a new commit on PR with this issue.
Thanks!
msg320880 - (view) Author: Ronald Oussoren (ronaldoussoren) * (Python committer) Date: 2018年07月02日 13:12
W.r.t. benchmarks: The full benchmark suite that's often used to assess the performance impact on real world code is: http://pyperformance.readthedocs.io
Running this takes some time, and the interesting bit is comparing the performance with and without your patch.
You could also test with a small script that uses the timeit module to run some code that performs a lot of function calls.
msg320907 - (view) Author: Dong-hee Na (corona10) * (Python committer) Date: 2018年07月02日 19:00
@ronaldoussoren
I updated the PR and we need some discussion.
I benchmarked new patch with Fibonacci codes.
baseline(master): 21.330535484 
without call sysctlbyname(PR 8046): 22.857963209 secs
with call sysctlbyname(PR 8046): 37.125129114 secs
So my choice is just believe pthread_get_stacksize_np() like rust do. (https://github.com/rust-lang/rust/blob/master/src/libstd/sys/unix/thread.rs#L246)
code:
def fib(n):
 if n == 0:
 return 0
 if n == 1:
 return 1
 return fib(n-2) + fib(n-1)
if __name__ == '__main__':
 import timeit
 print(timeit.timeit("fib(10)", setup="from __main__ import fib"))
msg321076 - (view) Author: Dong-hee Na (corona10) * (Python committer) Date: 2018年07月05日 05:39
@ronaldoussoren
I updated the PR
Please take a look.
msg321078 - (view) Author: Dong-hee Na (corona10) * (Python committer) Date: 2018年07月05日 06:08
benchmark result:
22.639114022999998 secs master (+0%)
22.806662271 secs PR 8046 (-0.74%)
def fib(n):
 if n == 0:
 return 0
 if n == 1:
 return 1
 return fib(n-2) + fib(n-1)
if __name__ == '__main__':
 import timeit
 print(timeit.timeit("fib(10)", setup="from __main__ import fib"))
msg321360 - (view) Author: Ronald Oussoren (ronaldoussoren) * (Python committer) Date: 2018年07月10日 07:30
Sorry about the slow response.
The patch looks OK, although I'm entirely happy yet about the workaround for the stack size on the main thread. Primarily because framework builds use a non-standard stack size as the default one is too small for the recursion limit (IIRC especially with debug builds). 
BTW. My own use case for this feature is primarily the use of python on threads created by the system (such as threads created by Cocoa APIs and libdispatch), for threads created in Python we can (and do) create them with a large enough stack (although this guard is helpful there too).
My next step will be to test the patch locally. This might take a while because I will be traveling the rest of this month. Worst case is that I'll report back during EuroPython 2018 (starting at 23th of July)
msg321643 - (view) Author: Dong-hee Na (corona10) * (Python committer) Date: 2018年07月14日 04:49
FYI, I've updated the patch to use rlim_cur() on the main thread when pthread_get_stacksize_np() is not accurate. This way works pretty well on my local machine.
Enjoy your travel!
msg347981 - (view) Author: Zackery Spytz (ZackerySpytz) * (Python triager) Date: 2019年07月15日 16:42
It seems that this issue is a duplicate of bpo-25518.
msg350497 - (view) Author: Dong-hee Na (corona10) * (Python committer) Date: 2019年08月26日 07:15
@ronaldoussoren
Do you have any ideas to resume this issue again?
msg388319 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021年03月08日 23:56
See also https://www.python.org/dev/peps/pep-0651/ 
History
Date User Action Args
2022年04月11日 14:59:02adminsetgithub: 78136
2021年03月08日 23:56:35vstinnersetmessages: + msg388319
2019年08月31日 18:18:01ned.deilylinkissue25518 superseder
2019年08月29日 00:13:29vstinnersetnosy: + vstinner
2019年08月26日 07:15:41corona10setmessages: + msg350497
2019年07月15日 16:42:20ZackerySpytzsetnosy: + ZackerySpytz
messages: + msg347981
2018年07月14日 04:49:12corona10setmessages: + msg321643
2018年07月10日 07:30:35ronaldoussorensetmessages: + msg321360
2018年07月05日 06:08:35corona10setmessages: + msg321078
2018年07月05日 05:39:53corona10setmessages: + msg321076
2018年07月02日 19:00:13corona10setmessages: + msg320907
2018年07月02日 13:12:03ronaldoussorensetmessages: + msg320880
2018年07月02日 13:04:24corona10setmessages: + msg320879
2018年07月02日 12:08:40ronaldoussorensetmessages: + msg320874
2018年07月02日 10:47:08corona10setmessages: + msg320868
2018年07月02日 09:54:38corona10setkeywords: + patch
stage: needs patch -> patch review
pull_requests: + pull_request7655
2018年07月01日 18:57:48ronaldoussorensetmessages: + msg320842
2018年06月30日 15:21:32corona10setnosy: + corona10
messages: + msg320786
2018年06月25日 11:18:15ronaldoussorencreate

AltStyle によって変換されたページ (->オリジナル) /