Improve the design of context switch #8220
Conversation
|
This looks awesome I saw your issue but did not know enough to really respond to it. Do you have any benchmarks of this change? |
firejox
force-pushed
the
fiber-context-refactor
branch
from
4ddcf68
to
1d6a385
Compare
|
I benchmark the threadring of kostya/crystal-benchmarks-game today. The new design performs slightly better than the origin one. Hence, the change will not cause the run-time overhead. |
|
Awesome thank you for the benchmarks. |
firejox
force-pushed
the
fiber-context-refactor
branch
from
1d6a385
to
5b183d3
Compare
firejox
force-pushed
the
fiber-context-refactor
branch
from
5b183d3
to
4ea9a71
Compare
|
I haven't been able to look at this in depth, yet. But it looks interesting. However, I'd ask to wait with this until after #7995 is merged. This is a performance improvement and new features have a higher priority. |
|
@straight-shoota I can wait for that pr because it is just a direction about what we can do. I also agree that making crystal work on windows is 🚀 . |
firejox
force-pushed
the
fiber-context-refactor
branch
from
4ea9a71
to
c2988c4
Compare
|
@firejox Could you please add back comments that describe the code's behaviour and reasoning behind it? Instead of removing the existing comments, maybe you can update them for the changes in this PR. This would help reviewing this PR and understanding the code later. Thanks. |
|
@straight-shoota Ok, I will add them back. |
firejox
force-pushed
the
fiber-context-refactor
branch
from
c2988c4
to
fc95d81
Compare
|
This looks really great, especially with the comments that even improve what we had before. However, I'm a bit overcharged with understanding everything that's going on. But I'm trusting in your expertise which seems to reach far further than mine in this regard 😄 It would be great if someone else could review this who is more familiar with the topic. Did you validate this is working as expected on native hardware? I've tested it on x86-64 both linux and windows. But especially the ARM variants might be bit tricky? |
|
Yes, this MUST be tested on real ARM and AArch64 hardware if it wasn't. QEMU isn't enough, and real hardware will immediately exhibit any issues —run the std spec suite for example. |
|
@straight-shoota At least some ARM variants should be pretty simple to test nowadays that Apple M1 is everywhere. (I don't have one though, so I can't do it). And we also have aarch64 variants of CI, but perhaps those are QEMU and thus not sufficient? |
|
Yeah, indeed the infrastructure situation has improved. Our CI workers run on native ARM machines, so that should be good. |
refer to #8193