Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move x registers from contexts to schedulers #943

Closed
wants to merge 1 commit into from

Conversation

pguyot
Copy link
Collaborator

@pguyot pguyot commented Nov 13, 2023

X registers are saved in context on context switching but are otherwise
allocated only once per scheduler thread, thus allowing up to 1024 registers
as the compiler allows

These changes are made under both the "Apache 2.0" and the "GNU Lesser General
Public License 2.1 or later" license terms (dual license).

SPDX-License-Identifier: Apache-2.0 OR LGPL-2.1-or-later

@pguyot pguyot force-pushed the w45/fix-xregs-limitation branch 2 times, most recently from 6d267c5 to fd9cc51 Compare November 13, 2023 22:02
Copy link
Collaborator

@bettio bettio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This approach is one of the 2 I was thinking about.
The other was offloading x regs > to a linked list, which is very slow, but it might be ok since they are not frequently used.
This approach has the con of using 4K for each executor thread, so 8K on ESP32.
Anyway I would like to benchmark this approach against other alternatives so we can make a good decision about it.
Last final thought on it: this is a big change in terms of C API, so we might target it for master.

@pguyot pguyot force-pushed the w45/fix-xregs-limitation branch 3 times, most recently from e68f880 to 913d23d Compare November 19, 2023 14:17
@pguyot
Copy link
Collaborator Author

pguyot commented Nov 19, 2023

This approach is one of the 2 I was thinking about. The other was offloading x regs > to a linked list, which is very slow, but it might be ok since they are not frequently used. This approach has the con of using 4K for each executor thread, so 8K on ESP32. Anyway I would like to benchmark this approach against other alternatives so we can make a good decision about it. Last final thought on it: this is a big change in terms of C API, so we might target it for master.

In 1c99c77 I propose to limit allocation to the number of x registers used by modules so far.

@pguyot pguyot force-pushed the w45/fix-xregs-limitation branch 2 times, most recently from fe240d8 to e08480b Compare November 22, 2023 07:20
@pguyot pguyot force-pushed the w45/fix-xregs-limitation branch 2 times, most recently from 27f087e to e018b56 Compare January 28, 2024 15:22
X registers are saved in context on context switching but are otherwise
allocated only once per scheduler thread, thus allowing up to 1024 registers
as the compiler allows

Signed-off-by: Paul Guyot <pguyot@kallisys.net>
@pguyot pguyot force-pushed the w45/fix-xregs-limitation branch from e018b56 to ff8fc11 Compare February 22, 2024 06:34
@pguyot pguyot marked this pull request as ready for review February 22, 2024 06:48
@pguyot pguyot closed this Feb 25, 2024
bettio added a commit that referenced this pull request Feb 27, 2024
This PR is focused into removing 2 main limitations:
1. Functions can have any number of parameters
2. Up to 1023 live x registers are supported

This restriction has been easily workarounded so far, but removing this
limitation closes a gap that makes development smoother.

Since so far everything worked nicely with just 16 registers, this
implementation threats "extended x registers" as an uncommon case, so using
them is way slower, but still possible, while reducing memory footprint to bare
minimum.

There is still a limitation: NIFs with more than 16 parameters are still not
allowed, but this limitation will be likely stay unnoticed.

This PR also fixes an unlikely corruption when having exactly 16 live registers,
and enables support of more than > 16 typed y registers (I never observed any
crash due to this limitation).

See also for #943 for an alternative approach, decoding is based on Paul's work
from that PR.

These changes are made under both the "Apache 2.0" and the "GNU Lesser General
Public License 2.1 or later" license terms (dual license).

SPDX-License-Identifier: Apache-2.0 OR LGPL-2.1-or-later
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants