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

Uncomment the code have 2 dependency with downstream projects #6672

Merged
merged 1 commit into from
Sep 7, 2022

Conversation

LinHu2016
Copy link
Contributor

@LinHu2016 LinHu2016 commented Aug 23, 2022

Remove the temporary comments after the dependent downstream project
has merged.

Update Method fixupForwardedSlot return true or false for the condition
to update heap object reference slot(we won't need to update the
reference slot if the reference is not fixed up) in order to avoid
potential reference slot overwriting(concurrent marking) during
Concurrent scavenger back out case.

Share method MM_Scavenger.shouldRememberSlot, remove duplicated logic
for checking if should remember the slot.

related: #6575
Signed-off-by: Lin Hu [email protected]

@amicic
Copy link
Contributor

amicic commented Sep 2, 2022

Let's address inner fixupForwardedSlot here, too. It should return a result (a bool or a new/fixed-up value), so that the outter fixupForwardedSlot updates the slot only if it really changed.

@amicic
Copy link
Contributor

amicic commented Sep 2, 2022

Remove 'depends on: eclipse-openj9/openj9#15603' comment from the commit message. In general, try to minimize references to downstream projects, including code comments.

@amicic
Copy link
Contributor

amicic commented Sep 2, 2022

You can do this simplification now: #6683 (comment)

@amicic
Copy link
Contributor

amicic commented Sep 6, 2022

You can do this simplification now: #6683 (comment)

This (shouldRememberSlot simplification) has not been done, yet.

/* Could happen if we aborted before completing RS scan */
return true;
}
if ((NULL != slotObjectPtr) && shouldRememberSlot(&slotObjectPtr)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

seems like shouldRememberSlot already checks if NULL

Remove the temporary comments after the dependent downstream project
has merged.

Update Method fixupForwardedSlot return true or false for the condition
to update heap object reference slot(we won't need to update the
reference slot if the reference is not fixed up) in order to avoid
potential reference slot overwriting(concurrent marking) during
Concurrent scavenger back out case.

Share method MM_Scavenger.shouldRememberSlot, remove duplicated logic
for checking if should remember the slot.


Signed-off-by: Lin Hu <[email protected]>
@LinHu2016
Copy link
Contributor Author

Hi @babsingh, could you please review and merge the PR? the related personal build has passed compiling, there are still some infra issues blocking testing(https://hyc-runtimes-jenkins.swg-devops.com/view/OpenJ9%20-%20Personal/job/Pipeline-Build-Test-Personal/14081/), Thanks

@babsingh
Copy link
Contributor

babsingh commented Sep 7, 2022

jenkins build all

@babsingh
Copy link
Contributor

babsingh commented Sep 7, 2022

jenkins build zos

1 similar comment
@babsingh
Copy link
Contributor

babsingh commented Sep 7, 2022

jenkins build zos

@babsingh
Copy link
Contributor

babsingh commented Sep 7, 2022

https://ci.eclipse.org/omr/job/PullRequest-zos_390-64/3810/

@jdekonin @AdamBrousseau zOS builds have been failing consistently due to the below error. Can you please take a look at the machine setup?

10:46:54   > /u/user1/jenkins-zos/git fetch --tags --progress [email protected]:eclipse/omr.git +refs/pull/6672/merge:refs/remotes/origin/pr/6672/merge # timeout=30
10:43:12  ERROR: Error cloning remote repo 'origin'
10:43:12  hudson.plugins.git.GitException: Command "/u/user1/jenkins-zos/git fetch --tags --progress [email protected]:eclipse/omr.git +refs/pull/6672/merge:refs/remotes/origin/pr/6672/merge" returned status code 128:
10:43:12  stdout: FOTS1370 Host key verification failed.
10:43:12  fatal: Could not read from remote repository.
10:43:12  
10:43:12  Please make sure you have the correct access rights
10:43:12  and the repository exists.

@AdamBrousseau
Copy link
Contributor

jenkins build zos

@AdamBrousseau
Copy link
Contributor

Both zos machines show this failure intermittently. Not sure why. I re-enabled #10 and your build passed on it.

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.

4 participants