Skip to content

Do not add Own.Sync. object to the list if scan wasn't successful #21857

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

Merged
merged 1 commit into from
May 13, 2025

Conversation

dmitripivkine
Copy link
Contributor

If Ownable Synchronizer object scan has not been successful (caused Copy Forward Abort) it should not be aadded to the list right away. This object is remembered in the work packet and is going to be rescanned for the second time.

@dmitripivkine
Copy link
Contributor Author

should fix #20395

@dmitripivkine
Copy link
Contributor Author

This is second attempt to fix the problem, first one was reverted. A few objects have been never added to the list during Copy Forward Abort/Global GC Continuation and were discovered by followed Copy Forward and trigger an assertion.
The problem was object was not added to the list during Copy Forward Abort but also never added during second path if it is located outside of Evacuate set.
So, correct logic should be:

  • always add object to the list if scan succeeds
  • never add object to the list is scan causes Copy Forward Abort
  • object is going to be discovered in Work Packet again and be added to the list unconditionally.

@dmitripivkine
Copy link
Contributor Author

jenkins test sanity xLinux,aix jdk21

@keithc-ca
Copy link
Contributor

Please correct the spelling of "successful".

@dmitripivkine dmitripivkine changed the title Do not add Own.Sync. object to the list if scan wasn't successfull Do not add Own.Sync. object to the list if scan wasn't successful May 12, 2025
If Ownable Synchronizer object scan has not been successful (caused Copy
Forward Abort) it should not be aadded to the list right away. This
object is remembered in the work packet and is going to be rescanned for
the second time.

Signed-off-by: Dmitri Pivkine <[email protected]>
@dmitripivkine
Copy link
Contributor Author

jenkins test sanity xLinux,aix jdk21

@amicic amicic merged commit 196082d into eclipse-openj9:master May 13, 2025
9 checks passed
amicic pushed a commit to amicic/openj9 that referenced this pull request May 20, 2025
If Ownable Synchronizer object scan has not been successful (caused Copy
Forward Abort) it should not be added to the list right away. This
object is remembered in the work packet and is going to be rescanned for
the second time.

This is a more correct variant of the first similar attempt to fix
eclipse-openj9#20589,
which would miss to add the object if originally scanned with PACKET
reason and failed during the scan.

It is also a more correct variant of the second attempt
eclipse-openj9#21857, which would
incorrectly add the object if originally scanned with DIRTY_CARD (as
Remembered Set, which is Root, not part of Collection Set) and failed
during the scan.

Signed-off-by: Aleksandar Micic <[email protected]>
amicic pushed a commit to amicic/openj9 that referenced this pull request May 20, 2025
If Ownable Synchronizer object scan (regardless if for PACKET or COPY
reason) has not been successful (caused Copy Forward Abort, when a child
object fails to copy) it should not be added to the list right away.
This object is remembered in the work packet and is going to be
rescanned for the second time.

The existing logic is incorrect for the case when the original scan
reason was PACKET (object still a part of Collection Set) and failed to
scan - the object would be added twice (both on the first and the second
scan isObjectInEvacuateMemoryNoCheck condition would be true).

This is a more correct variant of the first similar attempt to fix
eclipse-openj9#20589,
which would miss to add the object if originally scanned with COPY
reason and failed during the scan - the object would neither be added on
the first scan due to failed scanning nor be added on the second scan
due to not being a part of CS (it's copied, in survivor memory).

It is also a more correct variant of the second attempt
eclipse-openj9#21857, which would
incorrectly add the object if originally scanned with DIRTY_CARD (as
Remembered Set, which is Root, not part of Collection Set) and failed
during the scan.

Signed-off-by: Aleksandar Micic <[email protected]>
amicic pushed a commit to amicic/openj9 that referenced this pull request May 20, 2025
If Ownable Synchronizer object scan (regardless if for PACKET or COPY
reason) has not been successful (caused Copy Forward Abort, when a child
object fails to copy) it should not be added to the list right away.
This object is remembered in the work packet and is going to be
rescanned for the second time.

The existing logic is incorrect for the case when the original scan
reason was PACKET (object still a part of Collection Set) and failed to
scan - the object would be added twice (both on the first and the second
scan isObjectInEvacuateMemoryNoCheck condition would be true).

This is a more correct variant of the first similar attempt to fix
eclipse-openj9#20589,
which would miss to add the object if originally scanned with COPY
reason and failed during the scan - the object would neither be added on
the first scan due to failed scanning nor be added on the second scan
due to not being a part of CS (it's copied, in survivor memory).

It is also a more correct variant of the second attempt
eclipse-openj9#21857, which would
incorrectly add the object if originally scanned with DIRTY_CARD (as
Remembered Set, which is Root, not part of Collection Set) and failed
during the scan.

Signed-off-by: Aleksandar Micic <[email protected]>
amicic pushed a commit to amicic/openj9 that referenced this pull request May 20, 2025
If Ownable Synchronizer object scan (regardless if for PACKET or COPY
reason) has not been successful (caused Copy Forward Abort, when a child
object fails to copy) it should not be added to the list right away.
This object is remembered in the work packet and is going to be
rescanned for the second time.

The existing logic is incorrect for the case when the original scan
reason was PACKET (object still a part of Collection Set) and failed to
scan - the object would be added twice (both on the first and the second
scan isObjectInEvacuateMemoryNoCheck condition would be true).

This is a more correct variant of the first similar attempt to fix
eclipse-openj9#20589, which would miss
to add the object if originally scanned with COPY
reason and failed during the scan - the object would neither be added on
the first scan due to failed scanning nor be added on the second scan
due to not being a part of CS (it's copied, in survivor memory).

It is also a more correct variant of the second attempt
eclipse-openj9#21857, which would
incorrectly add the object if originally scanned with DIRTY_CARD (as
Remembered Set, which is Root, not part of Collection Set) and failed
during the scan.

Signed-off-by: Aleksandar Micic <[email protected]>
amicic pushed a commit to amicic/openj9 that referenced this pull request May 20, 2025
If Ownable Synchronizer object scan (regardless if for PACKET or COPY
reason) has not been successful (caused Copy Forward Abort, when a child
object fails to copy) it should not be added to the list right away.
This object is remembered in the work packet and is going to be
rescanned for the second time.

The existing logic is incorrect for the case when the original scan
reason was PACKET (object still a part of Collection Set) and failed to
scan - the object would be added twice (both on the first and the second
scan isObjectInEvacuateMemoryNoCheck condition would be true).

This is a more correct variant of the first similar attempt to fix
eclipse-openj9#20589, which would miss
to add the object if originally scanned with COPY reason and failed
during the scan - the object would neither be added on the first scan
due to failed scanning nor be added on the second scan due to not being
a part of CS (it's copied, in survivor memory).

It is also a more correct variant of the second attempt
eclipse-openj9#21857, which would
incorrectly add the object if originally scanned with DIRTY_CARD (as
Remembered Set, which is Root, not part of Collection Set) and failed
during the scan.

Signed-off-by: Aleksandar Micic <[email protected]>
amicic pushed a commit to amicic/openj9 that referenced this pull request May 20, 2025
If Ownable Synchronizer object scan (regardless if for PACKET or COPY
reason) has not been successful (caused Copy Forward Abort, when a child
object fails to copy) it should not be added to the list right away.
This object is remembered in the work packet and is going to be
rescanned for the second time.

The existing logic is incorrect for the case when the original scan
reason was PACKET (object still a part of Collection Set) and failed to
scan - the object would be added twice (both on the first and the second
scan isObjectInEvacuateMemoryNoCheck condition would be true).

This is a more correct variant of the first similar attempt to fix
eclipse-openj9#20589, which would miss
to add the object if originally scanned with COPY reason and failed
during the scan - the object would neither be added on the first scan
due to failed scanning nor be added on the second scan due to not being
a part of CS (it's copied, in survivor memory).

It is also a more correct variant of the second attempt
eclipse-openj9#21857, which would
incorrectly add the object if originally scanned with DIRTY_CARD (as
Remembered Set, which is Root, not part of Collection Set) and failed
during the scan.

Signed-off-by: Aleksandar Micic <[email protected]>
amicic pushed a commit to amicic/openj9 that referenced this pull request May 20, 2025
If Ownable Synchronizer object scan (regardless if for PACKET or COPY
reason) has not been successful (caused Copy Forward Abort, when a child
object fails to copy) it should not be added to the list right away.
This object is remembered in the work packet and is going to be
rescanned for the second time.

The existing logic is incorrect for the case when the original scan
reason was PACKET (object still a part of Collection Set) and failed to
scan - the object would be added twice (both on the first and the second
scan isObjectInEvacuateMemoryNoCheck condition would be true).

This is a more correct variant of the first similar attempt to fix
eclipse-openj9#20589, which would miss
to add the object if originally scanned with COPY reason and failed
during the scan - the object would neither be added on the first scan
due to failed scanning nor be added on the second scan due to not being
a part of CS (it's copied, in survivor memory).

It is also a more correct variant of the second attempt
eclipse-openj9#21857, which would
incorrectly add the object if originally scanned with DIRTY_CARD (as
Remembered Set, which is Root, not part of Collection Set) and failed
during the scan.

Signed-off-by: Aleksandar Micic <[email protected]>
amicic pushed a commit to amicic/openj9 that referenced this pull request May 20, 2025
If Ownable Synchronizer object scan (regardless if for PACKET or COPY
reason) has not been successful (caused Copy Forward Abort, when a child
object fails to copy) it should not be added to the list right away.
This object is remembered in the work packet and is going to be
rescanned for the second time.

The existing logic is incorrect for the case when the original scan
reason was PACKET (object still a part of Collection Set) and failed to
scan - the object would be added twice (both on the first and the second
scan isObjectInEvacuateMemoryNoCheck condition would be true).

This is a more correct variant of the first similar attempt to fix
eclipse-openj9#20589, which would miss
to add the object if originally scanned with COPY reason and failed
during the scan - the object would neither be added on the first scan
due to failed scanning nor be added on the second scan due to not being
a part of CS (it's copied, in survivor memory).

It is also a more correct variant of the second attempt
eclipse-openj9#21857, which would
incorrectly add the object if originally scanned with DIRTY_CARD (as
Remembered Set, which is Root, not part of Collection Set) and failed
during the scan, but would be added on the rescan as being with PACKET
reason (lost info that original scan was DIRTY_CARD).

Signed-off-by: Aleksandar Micic <[email protected]>
amicic pushed a commit to amicic/openj9 that referenced this pull request May 20, 2025
If Ownable Synchronizer object scan (regardless if for PACKET or COPY
reason) has not been successful (caused Copy Forward Abort, when a child
object fails to copy) it should not be added to the list right away.
This object is remembered in the work packet and is going to be
rescanned for the second time.

The existing logic is incorrect for the case when the original scan
reason was PACKET (object still a part of Collection Set) and failed to
scan - the object would be added twice (both on the first and the second
scan isObjectInEvacuateMemoryNoCheck condition would be true).

This is a more correct variant of the first similar attempt to fix
eclipse-openj9#20589, which would miss
to add the object if originally scanned with COPY reason and failed
during the scan - the object would neither be added on the first scan
due to failed scanning nor be added on the second scan due to not being
a part of CS (it's copied, in survivor memory).

It is also a more correct variant of the second attempt
eclipse-openj9#21857, which would
incorrectly add the object if originally scanned with DIRTY_CARD (as
Remembered Set, which is Root, not part of Collection Set) and failed
during the scan, but would be (incorrectly) added on the rescan as being
with PACKET reason (lost info that original scan was DIRTY_CARD).

Signed-off-by: Aleksandar Micic <[email protected]>
amicic pushed a commit to amicic/openj9 that referenced this pull request May 21, 2025
If Ownable Synchronizer object scan (regardless if for PACKET or COPY
reason) has not been successful (caused Copy Forward Abort, when a child
object fails to copy) it should not be added to the list right away.
This object is remembered in the work packet and is going to be
rescanned for the second time.

The existing logic is incorrect for the case when the original scan
reason was PACKET (object still a part of Collection Set) and failed to
scan - the object would be added twice (both on the first and the second
scan isObjectInEvacuateMemoryNoCheck condition would be true).

This is a more correct variant of the first similar attempt to fix
eclipse-openj9#20589, which would miss
to add the object if originally scanned with COPY reason and failed
during the scan - the object would neither be added on the first scan
due to failed scanning nor be added on the second scan due to not being
a part of CS (it's copied, in survivor memory).

It is also a more correct variant of the second attempt
eclipse-openj9#21857, which would
incorrectly add the object if originally scanned with DIRTY_CARD (as
Remembered Set, which is Root, not part of Collection Set) and failed
during the scan, but would be (incorrectly) added on the rescan as being
with PACKET reason (lost info that original scan was DIRTY_CARD).

Signed-off-by: Aleksandar Micic <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants