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

Remove duplicate definitions for J9VMSTATE_GC states from OpenJ9 #2787

Merged
merged 1 commit into from
Sep 14, 2018
Merged

Remove duplicate definitions for J9VMSTATE_GC states from OpenJ9 #2787

merged 1 commit into from
Sep 14, 2018

Conversation

dmitripivkine
Copy link
Contributor

All J9VMSTATE_GC state definitions have been moved to OMR, so remove
duplicate definitions from OpenJ9. Also rename
J9VMSTATE_GC_UNLOADING_DEAD_CLASSLOADERS to
J9VMSTATE_GC_CLEANING_METADATA

Signed-off-by: Dmitri Pivkine Dmitri_Pivkine@ca.ibm.com

@dmitripivkine
Copy link
Contributor Author

Jenkins test sanity pLinux jdk8

@DanHeidinga
Copy link
Member

Are there any DDR extensions that print this info? Is is known to the -Xjit:vmstate= commandline option?

@dmitripivkine
Copy link
Contributor Author

I believe there is no tool where GC States are decoded verbally. There is binary code reported only.
There might be an area of improvement in future

@dmitripivkine
Copy link
Contributor Author

Looking to Travis-ci build failure:

[ 74%] Building CXX object bcutil/test/dyntest/CMakeFiles/dyntest.dir/intern_tests.cpp.o
In file included from /home/travis/build/eclipse/openj9-openjdk-jdk9/build/linux-x86_64-normal-server-release/vm/bcutil/test/dyntest/intern_tests.cpp:35:0:
/home/travis/build/eclipse/openj9-openjdk-jdk9/build/linux-x86_64-normal-server-release/vm/gc_include/j9modron.h:32:27: fatal error: omrmodroncore.h: No such file or directory
 #include "omrmodroncore.h"
                           ^
compilation terminated.

@dmitripivkine
Copy link
Contributor Author

openj9/runtime/bcutil/test/dyntest/module.xml has path for omrmodroncore.h included:

			<include path="$(OMR_DIR)/gc/include" type="relativepath"/>

However it is failed with cmake

@dmitripivkine dmitripivkine force-pushed the master branch 2 times, most recently from 2da4268 to 927ff23 Compare September 12, 2018 18:15
@dmitripivkine
Copy link
Contributor Author

new include for omrmodroncore.h in j9modron.h is not necessary, this file is visible through GCExtensions.hpp already

@dmitripivkine dmitripivkine requested review from amicic and removed request for charliegracie September 12, 2018 18:22
@dmitripivkine
Remove duplicate definitions for J9VMSTATE_GC states from OpenJ9
50a2d7f 
All J9VMSTATE_GC state definitions have been moved to OMR, so remove
duplicate definitions from OpenJ9. Remove duplicate definitions for Card
Table States. Replace j9modron.h to omrmodroncore.h in includes in Tree
Evaluator code for Arm, P and Z (use CARD_DIRTY). Also rename
J9VMSTATE_GC_UNLOADING_DEAD_CLASSLOADERS to
J9VMSTATE_GC_CLEANING_METADATA in GC code
 
Signed-off-by: Dmitri Pivkine <Dmitri_Pivkine@ca.ibm.com>
@dmitripivkine
Copy link
Contributor Author

Jenkins test sanity pLinux jdk8

@dmitripivkine
Copy link
Contributor Author

dmitripivkine commented Sep 13, 2018 •
edited
Loading

Code compiles successfully for Linux ARM, Linux PPC and ZOS in personal builds.

@dmitripivkine
Copy link
Contributor Author

@fjeremic J9TreeEvaluator.cpp for P, Z and ARM use CARD_DIRTY definition and include j9modron.h to get it. However this definition (with many others) has been moved to omrmodronbase.h
(but j9modron.h use to have duplicate for it). I am removing duplicates so replace include statement to point to new location. Looks like there is no problem with include path - compilation works for all three platforms. I tested it in personal builds. Would you please review JIT part of this change?

@dmitripivkine
Copy link
Contributor Author

I forgot to mention that path to location of omrmodronbase.h is already added to jitinclude.mk:
$(OMR_DIR)/gc/include

@fjeremic fjeremic self-assigned this Sep 13, 2018
@fjeremic
Copy link
Contributor

Jenkins test sanity all JDK8

@fjeremic fjeremic merged commit 50c701f into eclipse-openj9:master Sep 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fjeremic fjeremic fjeremic approved these changes

@amicic amicic amicic approved these changes

Assignees

@fjeremic fjeremic

Labels
comp:gc
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@dmitripivkine @DanHeidinga @fjeremic @amicic