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

ensure write barrier in last step of vector-sort! #784

Merged
merged 1 commit into from
Dec 21, 2023
Merged

Conversation

mflatt
Copy link
Contributor

@mflatt mflatt commented Dec 21, 2023

The old $vector-copy! could use $ptr-copy!, which doesn't have an associated write barrier. As a result, using $vector-copy! at the end of vector-sort! is incorrect.

The new test is arranges for one half of a vector to have generation-0 objects and the other to have immediate objects; the two halves are already sorted, so they end up being swapped via a temporary vector and $vector-copy!. As a result, a collection afterward can fail to scan the half of the original vector that it should scan.

The solution here is to rename the internal $vector-copy! function to $vector-fill-copy! (by analogy to $stencil-vector-fill-set!) and use keep using it for things like vector-copy. But the new $vector-copy! as used by vector-sort! avoids $ptr-copy!.

The old `$vector-copy!` could use `$ptr-copy!`, which doesn't have an
associated write barrier. As a result, using `$vector-copy!` at the
end of `vector-sort!` is incorrect.

The new test is arranges for one half of a vector to have generation-0
objects and the other to have immediate objects; the two halves are
already sorted, so they end up being swapped via a temporary vector
and `$vector-copy!`. As a result, a collection afterward can fail to
scan the half of the original vector that it should scan.

The solution here is to rename the internal `$vector-copy!` function
to `$vector-fill-copy!` (by analogy to `$stencil-vector-fill-set!`)
and use keep using it for things like `vector-copy`. But the new
`$vector-copy!` as used by `vector-sort!` avoids `$ptr-copy!`.
Copy link
Contributor

@burgerrg burgerrg left a comment

Choose a reason for hiding this comment

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

That was a great find!

@mflatt mflatt merged commit 056efe1 into cisco:main Dec 21, 2023
13 checks passed
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