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

GC: Use upstream patch for green threads support #8225

Merged
merged 2 commits into from Sep 26, 2019

Conversation

@bcardiff
Copy link
Member

commented Sep 24, 2019

This requires also an update of the distribution-scripts (reason why this PR is draft).
A compiler with the updated GC 8.0.4 + upstream patch will be available in circleci for a smoke testing. So far it seems to work locally.

The preview_mt jobs will fail with compilation issues since the 0.31.0 has the old patch bundled.

As of the changes itself, bdw-gc patch ended returning an opaque thread handler that is stored now in Thread.gc_thread_handler and a StackBase structure that is the stackbottom in most architectures. In IA-64 includes the reg_base, but we don't support IA-64. Since the fibers only knows about the stackbottom, I preferred to leave the reg_base commented.

Depends on: crystal-lang/distribution-scripts#47

Fixes: #8213

@bcardiff

This comment has been minimized.

Copy link
Member Author

commented Sep 25, 2019

A build with the upstream patch is available in snap refresh crystal --channel=edge/ci-update-gc-dev.

Unsigned rpm, deb packages and tar.gz can be downloaded from https://circleci.com/gh/crystal-lang/crystal/29489#artifacts/containers/0 in case someone is willing to smoke on their environment.

@bcardiff bcardiff force-pushed the ci/update-gc branch from 5b4319f to 883e8a1 Sep 25, 2019
@bcardiff bcardiff marked this pull request as ready for review Sep 25, 2019
@bcardiff

This comment has been minimized.

Copy link
Member Author

commented Sep 25, 2019

On my side the smoke test went well. I've run the benchmarks and nothing explodes, memory consumption was similar than before.

If someone else wants to confirm/try and review the changes we can move forward here.

@bararchy

This comment has been minimized.

Copy link
Contributor

commented Sep 25, 2019

@bcardiff works for us 👍

@@ -17,6 +17,13 @@ lib LibGC
alias SizeT = LibC::SizeT
alias Word = LibC::ULong

struct StackBase
mem_base : Void*
# reg_base : Void* should be used also for IA-64 when/if supported

This comment has been minimized.

Copy link
@RX14

RX14 Sep 25, 2019

Member

We will never support IA-64, the architecture is thoroughly dead.

@RX14
RX14 approved these changes Sep 25, 2019
@anatol

This comment has been minimized.

Copy link
Contributor

commented Sep 26, 2019

I tested this patch with upstream GC changes at Arch Linux and it looks good. Thank you for the work.

One the patch lands crystal repo I'll push a new Arch crystal package.

@bararchy

This comment has been minimized.

Copy link
Contributor

commented Sep 26, 2019

@bcardiff seems both @anatol and our team verified it's working, this will really help up streamline development.
Can we cut a new version? 🙏

@bcardiff bcardiff merged commit cdafa0e into master Sep 26, 2019
18 of 19 checks passed
18 of 19 checks passed
ci/circleci: test_preview_mt Your tests failed on CircleCI
Details
ci/circleci: check_format Your tests passed on CircleCI!
Details
ci/circleci: dist_artifacts Your tests passed on CircleCI!
Details
ci/circleci: dist_darwin Your tests passed on CircleCI!
Details
ci/circleci: dist_docker Your tests passed on CircleCI!
Details
ci/circleci: dist_docs Your tests passed on CircleCI!
Details
ci/circleci: dist_linux Your tests passed on CircleCI!
Details
ci/circleci: dist_linux32 Your tests passed on CircleCI!
Details
ci/circleci: dist_snap Your tests passed on CircleCI!
Details
ci/circleci: prepare_common Your tests passed on CircleCI!
Details
ci/circleci: prepare_maintenance Your tests passed on CircleCI!
Details
ci/circleci: publish_docker Your tests passed on CircleCI!
Details
ci/circleci: publish_snap Your tests passed on CircleCI!
Details
ci/circleci: test_darwin Your tests passed on CircleCI!
Details
ci/circleci: test_dist_linux_on_docker Your tests passed on CircleCI!
Details
ci/circleci: test_linux Your tests passed on CircleCI!
Details
ci/circleci: test_linux32 Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@bcardiff bcardiff deleted the ci/update-gc branch Sep 27, 2019
bcardiff added a commit that referenced this pull request Sep 27, 2019
* Use bdw-gc upstream mt patch
* Update distribution-scripts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.