Remove the interface function fullCollectNoStack from the gcinterface

[schveiguy]

This is an experiment to see whether anything will fail if we completely remove the (seemingly useless/dangerous) fullCollectNoStack.

OK, no longer an experiment. I think we should remove this function.

I probably should write a changelog...

See the investigation I did here: https://forum.dlang.org/post/[email protected]

[dlang-bot]

Thanks for your pull request, @schveiguy!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + dmd#16401"

[schveiguy]

Just so I can record the stuff I learned while researching this:

The original D1 GC only called this branch of the code (what was the equivalent of fullCollectNoStack) on termination.

The original fullCollectNoStack ONLY skipped scanning the stack if there was 1 thread.

I believe the idea was that since the gc is being removed anyway, and the only place in a single-threaded program where this GC is being terminated is from the main thread after main has exited, we can skip all stack scanning since the program is about to die.

It's kind of foolish in one sense -- if this is the case the main thread stack is tiny, and there are no other stacks to scan.

Over the years as the code has evolved, comments were incorrectly added in the wrong place, making it seem like the current code is necessary, but I think it is not. I believe there is no harm in scanning additional roots. And since D1, the fullCollectNoStack has been rewritten to actually avoid scanning stacks even in a multi-threaded environment.

[thewilsonator]

This otherwise looks good, modulo any further CI failures.

[thewilsonator]

maybe add a GC.collect(); here to force a collection then?

[schveiguy]

I'm not seeing why we have to force one when it's going to collect at the end of the program anyway. The goal here is to make sure there is at least one piece of garbage that will trigger the error, and making 100 of them means at least one of those won't be referred to in stack/registers.

Probably could be 2, but 100 is cheap...

[schveiguy]

What's the deal with the coverage upload failures? Are those normal?

[thewilsonator]

No idea. Yes.

[rikkimax]

I'm sick of the coverage upload failures.

It should be removed, there are no acceptable solutions for public repositories to fix this issue.

https://community.codecov.com/t/upload-issues-unable-to-locate-build-via-github-actions-api/3954

[schveiguy]

A question for reviewers, is a deprecation cycle needed for this? I seriously doubt anyone even thinks about using the GC interface. But it is there...

[thewilsonator]

I probably should write a changelog

That would be good

[thewilsonator]

Apart from a lack of Changelog entry, this looks good. I don't think that a deprecation period is needed.

[schveiguy]

changelog added.