implement switching using the Stackman submodule #230
Conversation
|
Cool, I'll have a look at it within the next few days. |
|
Note that the current pr overrides existing stackless code where the same
platforms are supported by both techs. That was easier for testing but
maybe not a good default initially.
…On Tue, 7 Jan 2020, 10:19 Anselm Kruis, ***@***.***> wrote:
Cool, I'll have a look at it within the next few days.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#230?email_source=notifications&email_token=ABN3FRYLPM75Z3T4S3JWOUTQ4RJKXA5CNFSM4KCOHRG2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEIIMTVA#issuecomment-571525588>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABN3FR7MRVZIVEYROXI3XMDQ4RJKXANCNFSM4KCOHRGQ>
.
|
|
How is stackman related to your previous work in issue #10 and https://bitbucket.org/stackless-dev/tealet/src/default/ ? Additionally there's still a bug somewhere: I compliled the code with test_raise_exception_H (test_miscell.TestSwitchTrap) ... Fatal Python error: Segmentation fault Current thread 0x00007f85db4ac140 (most recent call first): |
|
I've changed it to https, but the "stackman" project is currently owned by me. It is a public repository. We can discuss transfering it to the stackless project, if you like this approach. I think it would be a good idea in general to decouple the stack manipulation in the way that i'm doing. |
|
Hi Anselm. For a bit of background:
So, it is interesting that we get a crash with clang. It is possible that the assembly code generator doesn't work with that, I didn't test it. There are unittests for stackman, but I don't have CI yet. I'll investigate, and set up a CI suite to test a set of platforms. So, some things I need to fix, so please give me a few days to look into that. |
|
Odd. I can repro this, It happens during hard switching for test_raise_exception. after TASKLET_CLAIMVAL() in line 1192 in scheduling.c, retval is NULL. It seems to be because the prev pointer is bogus. I'll look into this. |
|
Ok, it turns out that clang doesn't respect the same attr((optimize)) that gcc does. In particular, I cannot tell it to skip frame pointers, or perform optimization. I can see if I can get clang to work correctly, at least on x64. It is unfortunate that I cannot mandate a certain optimization level for a function using attr. for example, with -O1, it will perform a tail call for the second call to the callback, something which I am unsure if is valid! For now, I have to make extra checks for clang. |
|
Hi, now the --pydebug tests pass, at least for x86_64 on my old computer at home. It was by pure chance, that I used clang for my very first test. I appreciate your work very much. We should replace the old Stackless code by stackman once it is sufficiently mature. A few comments about your patch About optimisation and stack switching: the Stackless stack switching code relies on the respective ABI (application binary interface) specification in order to know register usage and stack layout. Unfortunately the ABI specification does not apply to inlined functions and every C compiler is free to inline a function except in one single case: if the compiler does not know the called function. Fortunately the C standard guarantees to read the value of a volatile variable from memory on each access. If you call a function using a volatile pointer, the compiler must not infer the called function from the initialisation of the pointer and therefore the compiler can't inline the function. See #183 for an example. This consideration might apply to stackman too. The Python Contributor Licensing Agreement requires, that the initial licence of a patch is on of Academic Free License v. 2.1 or Apache License, Version 2.0. Your stackman code uses the MIT licence. I wonder, if this could cause problems? Starting with commit f653fd4 (bpo-35296) Makefile.pre.in contains a list of all header files. You should add stackman headers to this list. And we need to make sure, that the official source archive of Stackless contains a copy of the required stackman files. I use the script Stackless/Tools/create_source_archive.sh to create the archive. Probably it needs a patch. About the stackman repository: once stackman is stable we could include a copy of the files in the stackless repository. Otherwise I would prefer a clone under https://github.com/stackless-dev/stackman. There is always a risk, that a private github repository disappears. Finally the change needs to be documented in Stackless/changelog.txt. Eventually the section "Building Stackless" in Stackless/readme.txt needs an update too (regarding the git submodule usage). |
|
Hi Anselm, thanks for your comments. Regarding ABI, I have been careful to use the documented platform ABI whenever possible. I have changed the x86-64 on unix to save the floating point status variables as already done in the stackless code. The main change stackman does is that the platform function is essentially fixed, and the implementation is from the callback. If I understand you correctly, you are concerned that a compiler, using whole program optimization, might bypass the function call-by-pointer and instead inline the function in the stackman_switch() function. But having a callback also adds a different facility: The stackman_switch() function is fixed, regardless of the application. This allows us to alternatively implement it using pure assembly. And in fact, this is how I have solved the clang problem for stackless. We can use gcc to generate the assembly code (using gcc -S), verify that it correct, and then use this assembler in applications that are created by unix-like toolchains. I will add a macro to stackman to prefer assembly implementation over inline assembly, if both are available. I will also generate the correct .S files for the supplied platforms. Windows is, of course, different, since the .sln files need to be changed. Also, windows assembler doesn't do C Preprocessing (as cc does with .S files) I'll see about making the callback pointer volatile. I'll look at your other suggestsins too (its been quite long since I worked on Stackless, my mind gets rusty). I'll also be happy to transfer ownership of this to the Stackless team, or we can clone it into there, whatever. As for the license, it was my understanding that the MIT was the most permissive license there is. I can check if adding MIT licensed code into python/stackless is problematic. Otherwise, I'm fine with changing the license. |
|
I read up on #183 and under stand your concern. I.e. if the entire function is inlined then the generated inline assembly code no longer works. |
Not a lawyer here, but my understanding of Apache main difference is the implied protection from of possible patents others may have on the contributed code. This means that, even the code being opensource, MIT license could allow someone to patent software and sue users. See: https://snyk.io/blog/mit-apache-bsd-fairest-of-them-all/
In common terms, patents sue for Apache code leads to prohibition to use any other Apache code 😎 |
|
So, this PR is next on my list. :-) |
Use the official repository.
The outcome does compile, but fails to link.
|
Hi Kristján, I updated your pull request. It now uses current HEAD from https://github.com/stackless-dev/stackman. It compiles, but fails to link: |
|
Yes. The new architecture of 'stackman' uses pre-compiled binaries as a preferred way, to try to eliminate the assembly steps otherwise required. The functions are dead simple and stable and should rarely change, hence are commited in the repo. Target projects, such as stackless, need to link with libstackman and put the proper architecture path in the library search path. |
|
Ok, that explains a lot. I have some concerns to depend on pre-compiled software. In my professional work policies make it difficult to integrate pre-compiled open source software into our products. If I use a pre-compiled component, I need a contract with some "vendor" to ensure maintenance and security updates. |
|
It is optional, you can elect to include the necessary sources. The
problem is that it is usually a hassle to invoke the assembler as part of a
make script. For python plugins, for example, there is no way to
automatically start assembly. Assembly is usually very un-portable.
You can also use in-line assembler, but that has its own problems, the
output from that is not consistent. This is the reason for lots of the
hacks that have been present around the stack switching throughout the
years, the special hacks to avoid inlining, and so on and so forth.
The source code is there, and there is simply an assembled object file,
guaranteed to work, which is provided along side.
You will notice that this is also the approach that is generally used in
'gevent'. That library just has the pre-compiled .obj files included so
that users of the lib don't have to go through the problem of setting up
assembler / in-line assembly.
mið., 23. jún. 2021 kl. 12:09 skrifaði Anselm Kruis <
***@***.***>:
… Ok, that explains a lot.
I have some concerns to depend on pre-compiled software. In my
professional work policies make it difficult to integrate pre-compiled open
source software into our products. If I use a pre-compiled component, I
need a contract with some "vendor" to ensure maintenance and security
updates.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#230 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABN3FRZ6MVDLIXHLOTQ5J5DTUHFHLANCNFSM4KCOHRGQ>
.
|
|
I opened #278 to discuss alternative approaches to utilize Stackman. |
|
It is possible that there is an extra function call level now or something, but I can't remember how these sizes were computed. what units are they? If we ever move to using "libtealet" however, I would expect differences in performance, in either direction, because there, stack memory management is different. See |
|
The "make clean" issue is resolved. Next step: make a plain "./configure" without any arguments work again. Questions:
|
We need a way to force a build without stackman. Using the --with-MODULE mechanism of autoconf we can even configure the location of the Stackman directory. Move the Stackman directory to Stackless/stackman.
Actually we extract the object files and link the object files, because this is much simpler to integrate into the Python Makefile.
|
On Linux / Unix the patch is now probably OK. On Windows, it is still completely untested. And the changelog entry is still missing. |
You can set the property stackmanDir=no to build without Stackman. You can set the property stackmanDir=C:\path\to\stackman to build with Stackman installed in C:\path\to\stackman. (To set a property add '"/p:name=value"' to the build.bat command line.
| <AdditionalDependencies>version.lib;shlwapi.lib;ws2_32.lib;$(stackmanLib)%(AdditionalDependencies)</AdditionalDependencies> | ||
| <AdditionalLibraryDirectories Condition="'$(stackmanLib)'!='' And '$(Platform)'=='x64'">$(stackmanDir)lib\win_x64;%(AdditionalLibraryDirectories)</AdditionalLibraryDirectories> | ||
| <AdditionalLibraryDirectories Condition="'$(stackmanLib)'!='' And '$(Platform)'=='Win32'">$(stackmanDir)lib\win_x86;%(AdditionalLibraryDirectories)</AdditionalLibraryDirectories> | ||
| <ImageHasSafeExceptionHandlers Condition="'$(stackmanLib)'!='' And '$(Platform)'=='Win32'">false</ImageHasSafeExceptionHandlers> |
There was a problem hiding this comment.
@kristjanvalur : Currently this is required on x86. Otherwise I get the error message:
stackman.lib(switch_x86_msvc.obj) : error LNK2026: module unsafe for SAFESEH image. [C:\build\stackless38\PCbuild\pythoncore.vcxproj] Creating library C:\build\stackless38\PCbuild\win32\python38_d.lib and object C:\build\stackless38\PCbuild\win32\python38_d.exp C:\build\stackless38\PCbuild\win32\python38_d.dll : fatal error LNK1281: Unable to generate SAFESEH image.[C:\build\stackless38\PCbuild\pythoncore.vcxproj]
|
I'll merge this pull request now, because I made good progress with merging C-Python commits (v3.8.0a3) is merged. In my opinion it is better to merge this patch now and fix problems later on. |
|
Hi, I've been on vacation and not following closely. |
The Stackman project (http://github.com/kristjanvalur/stackman) aims to provide simple
stack switching code for modern platforms. The aim is to have application-agnostic code that
performs the basic functionality of saving machine registers and switching stack pointer, leaving the custom code of saving/restoring stack and choosing stack pointer to a user callback.
The resulting switching code is very succinct and easy to verfy.
This pull requests adds Stackman based switching for the supported platforms, which is GCC on
intel (x86 and x86_64) and ARM 32 hard float and aarch64.