Skip to content

Conversation

@pitrou
Copy link
Member

@pitrou pitrou commented Nov 21, 2023

What changes are included in this PR?

Our codebase has many uses of unsafe.Pointer. It may be a good idea to add debug checks for these uses in CI builds, especially where data crosses C boundaries.

Are these changes tested?

Yes.

Are there any user-facing changes?

No.

@github-actions
Copy link

⚠️ GitHub issue #38824 has been automatically assigned in GitHub to PR creator.

@github-actions github-actions bot added the awaiting review Awaiting review label Nov 21, 2023
@pitrou
Copy link
Member Author

pitrou commented Nov 21, 2023

I wonder if this is making Go CI builds slower than they were. I see a ton of unsafe.Pointer usage in our Go codebase.

@zeroshade

@pitrou pitrou marked this pull request as ready for review November 21, 2023 16:58
@pitrou
Copy link
Member Author

pitrou commented Nov 21, 2023

I wonder if this is making Go CI builds slower than they were. I see a ton of unsafe.Pointer usage in our Go codebase.

Answer: CI builds seem roughly the same duration as before.

@pitrou pitrou requested a review from zeroshade November 21, 2023 17:02
@pitrou pitrou force-pushed the gh38824-go-gcflags branch from 3b9143b to 79e0988 Compare November 23, 2023 15:14
@pitrou
Copy link
Member Author

pitrou commented Nov 23, 2023

@github-actions crossbow submit -g go

@github-actions
Copy link

Revision: 29bf1db

Submitted crossbow builds: ursacomputing/crossbow @ actions-ed99c57b77

Task Status
test-debian-11-go-1.19 Azure
test-debian-11-go-1.21 Azure

Copy link
Member

@zeroshade zeroshade left a comment

Choose a reason for hiding this comment

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

LGTM thanks!

@github-actions github-actions bot added awaiting merge Awaiting merge and removed awaiting review Awaiting review labels Nov 27, 2023
@pitrou pitrou merged commit 1cd22df into apache:main Nov 27, 2023
@pitrou pitrou removed the awaiting merge Awaiting merge label Nov 27, 2023
@pitrou pitrou deleted the gh38824-go-gcflags branch November 27, 2023 16:53
@conbench-apache-arrow
Copy link

After merging your PR, Conbench analyzed the 3 benchmarking runs that have been run so far on merge-commit 1cd22df.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details.

dgreiss pushed a commit to dgreiss/arrow that referenced this pull request Feb 19, 2024
### What changes are included in this PR?

Our codebase has many uses of `unsafe.Pointer`. It may be a good idea to add debug checks for these uses in CI builds, especially where data crosses C boundaries.

### Are these changes tested?

Yes.

### Are there any user-facing changes?

No.

* Closes: apache#38824

Authored-by: Antoine Pitrou <antoine@python.org>
Signed-off-by: Antoine Pitrou <antoine@python.org>
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.

[Go] Enable GC checks

2 participants