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.

Sign up for GitHub

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

Jump to bottom

add (failing) tests of liveslots GC sensitivity #7158

Merged
merged 2 commits into from
Mar 17, 2023
Merged

add (failing) tests of liveslots GC sensitivity #7158

merged 2 commits into from
Mar 17, 2023

Conversation

warner
Copy link
Member

@warner warner commented Mar 11, 2023

This adds some (failing) tests to demonstrate how liveslots makes syscalls in a way that is sensitive to GC activity, which is not what we want. The three new tests are marked with test.failing, so the failures are tolerated by CI, until we get the core issues fixed (#7142, #6360).

It also factors a mockGC test harness out of test-liveslots.js so it can be used more widely.

@warner warner added the liveslots requires vat-upgrade to deploy changes label Mar 11, 2023
@warner warner requested a review from gibson042 March 11, 2023 04:26
@warner warner self-assigned this Mar 11, 2023
@datadog-full-agoric
Copy link

datadog-full-agoric bot commented Mar 11, 2023 •
edited
Loading

Datadog Report

Branch report: 7142-gc-sensitivity
Commit report: 0a3cf6a

✅ agoric-sdk: 0 Failed, 0 New Flaky, 3757 Passed, 53 Skipped, 18m 30.27s Wall Time

FUDCo
FUDCo approved these changes Mar 17, 2023
View reviewed changes
Copy link
Contributor

@FUDCo FUDCo left a comment

Choose a reason for hiding this comment

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

Looks good.

I think this is the first time I viscerally believe the claims of GC visibility happening -- I mean, I believed it intellectually just from the fact that it seemed to be happening, but this let's us see the culprit at work and makes it real for me in a way that hitherto had not been the case.

warner added 2 commits March 16, 2023 18:05
@warner
extract mock-gc.js out from test-liveslots.js
fca9d6b 
This utility creates manually-controlled GC utilities: WeakRef and
FinalizationRegistry
@warner
add failing tests: syscalls GC sensitivity via reanimation
d3f63be 
This exercises three cases in which syscalls are sensitive to GC
activity:

* reanimateDurableKindID, when regenerating a KindHandle (#7142)
* reanimate, when regenerating a plain Representative (#7142)
* reanimateCollection, when regenerating a virtual collection (#6360)
@warner warner force-pushed the 7142-gc-sensitivity branch from 655773e to d3f63be Compare March 17, 2023 01:05
@warner warner removed the request for review from gibson042 March 17, 2023 01:09
@warner warner added the automerge:rebase Automatically rebase updates, then merge label Mar 17, 2023
@mergify mergify bot merged commit d5d2624 into master Mar 17, 2023
@mergify mergify bot deleted the 7142-gc-sensitivity branch March 17, 2023 01:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@FUDCo FUDCo FUDCo approved these changes

Assignees

@warner warner

Labels
automerge:rebase Automatically rebase updates, then merge liveslots requires vat-upgrade to deploy changes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@warner @FUDCo