-
Notifications
You must be signed in to change notification settings - Fork 6.4k
8317279: Standard library implementation of value classes and objects #31123
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
base: master
Are you sure you want to change the base?
Changes from all commits
383d50e
8997310
f29dba8
7cb66d6
2f27999
37733a5
b785dcd
3b345f3
fab935b
023e8dc
59cf255
59347b1
00f5e96
9525f8c
ffb47cf
8b2263f
d841d4a
2c87eee
efdf137
8694152
e391f76
e12473c
d6bc57f
1ea75fa
f09dea6
ff518b2
6d2c579
bab0024
5dbc217
cfda30f
583bdbc
2df83c2
4495013
af54dff
6dbddf4
fcfbcfc
301fe4a
c71f69c
4b9ee21
57dc71c
4f48eae
a9dfc23
5814c9d
5131e8a
d886298
d14b4dd
2bc2fe3
8d84aee
0eef773
555fbde
58efbec
6391715
7f562b3
46aac16
7558cd7
46dafa3
4b95be2
31ee5a8
beec164
cadae12
1c746e9
5ad1577
cbb23bb
e946a61
be1bc9a
1dedf05
737cf51
ce13625
800190d
ef03842
260fe04
7ec5a18
0432d28
632b1bd
9a8e822
873f02d
f39fa97
e51da80
2585704
96a8b4b
4990f91
7c2d2cd
2d330b1
de6aff4
6b9eb7d
cc40808
4824ac0
37504c6
50ed255
687c91c
b5347d2
5758540
530e74c
8477926
b13d505
15e9bb6
7f751ee
381cedf
1f15e5a
af9a023
0cee7f0
e08d163
f87d6d1
473248e
5bd51e2
2060561
809e1cd
67240cc
c5fafa9
a63fc22
679d759
d07da49
cbb983c
47f6119
0fd28c1
0dc5f67
b4abeb6
4a8ca20
20105ed
71f4b08
d7bb464
884d529
b3b4a2c
36acdd6
53268eb
0da436e
4c23f0e
0f58277
5ccd489
931493b
10ca1d3
19cf14f
8a869ae
703ac96
601e48e
804858f
dad467a
193b6a8
3086665
6a7073f
18449ec
b9b9eef
100e344
66dddaf
059baea
9e80425
194c6e8
071157b
4c1a52b
a974e5e
39e74d0
5c8673b
ed3bfbc
afbab0d
69d84f3
879523a
ce940c6
2af456d
8e9d0a5
5f1405f
ea6ec3d
03a2847
702e092
0639bc3
74f27eb
a7567f2
2cbfd78
022f70b
2ad1916
a3a6dda
2eae815
8ba03de
993c879
168f56b
3246309
246ffbf
fe2308a
2c4528c
ef794af
da99de9
fb95aaa
30d67a2
418ecb7
b466279
72a74ed
af33332
6d95a41
95bef16
042a35d
1ca1c8d
26c8f2e
4d6d288
abebcaa
c2f67c7
b3a718e
2fe010c
7ab2d1e
d15e8b7
28a9f6e
642ef0a
6eb965b
1badaaa
8e1cb51
43640ba
6663aa3
347b36f
a339024
9cd13e6
57b668a
821c522
892d502
7974808
ed61dc2
426cc06
2523a4f
6931557
246c6b3
fce1837
ce9ae18
d4a3b6b
28e1ab5
dd93655
1eb4aaa
d09a203
f234daf
38e9595
4c1c438
d0b6c69
a8cc7b3
d85e818
5e57521
3923e0c
8d02455
8f8b311
5e32888
d4fc4b2
84422ff
af3e804
39190ed
441b428
c93431c
e614970
4542f1d
42fc9c9
59019cf
b77db44
6f7797c
f0a7ef2
5ee106a
cdbe55a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -794,6 +794,20 @@ ifeq ($(BUILD_JTREG_TEST_THREAD_FACTORY), true) | |
| )) | ||
| endif | ||
|
|
||
| # Builds the value class plugin jtreg extension (JEP 401) | ||
| $(eval $(call SetupTarget, build-test-value-class-plugin, \ | ||
| MAKEFILE := test/BuildJtregValueClassPlugin, \ | ||
| TARGET := build, \ | ||
| DEPS := interim-langtools exploded-image, \ | ||
| )) | ||
|
|
||
| # Copies the value class plugin into the test image | ||
| $(eval $(call SetupTarget, test-image-value-class-plugin, \ | ||
| MAKEFILE := test/BuildJtregValueClassPlugin, \ | ||
| TARGET := images, \ | ||
| DEPS := build-test-value-class-plugin, \ | ||
| )) | ||
|
|
||
| $(eval $(call SetupTarget, build-microbenchmark, \ | ||
| MAKEFILE := test/BuildMicrobenchmark, \ | ||
| DEPS := interim-langtools exploded-image build-test-lib, \ | ||
|
|
@@ -1310,6 +1324,8 @@ ifeq ($(BUILD_JTREG_TEST_THREAD_FACTORY), true) | |
| test-image: test-image-test-thread-factory | ||
| endif | ||
|
|
||
| test-image: test-image-value-class-plugin | ||
|
|
||
|
Comment on lines
+1327
to
+1328
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This could be moved to line 1303-1306 where all the other non conditional prereqs for test-image are declared. |
||
| ifneq ($(JMH_CORE_JAR), ) | ||
| test-image: build-microbenchmark | ||
| endif | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -93,6 +93,9 @@ JTREG_FAILURE_HANDLER := $(JTREG_FAILURE_HANDLER_DIR)/jtregFailureHandler.jar | |
| JTREG_TEST_THREAD_FACTORY_DIR := $(TEST_IMAGE_DIR)/jtreg_test_thread_factory | ||
| JTREG_TEST_THREAD_FACTORY_JAR := $(JTREG_TEST_THREAD_FACTORY_DIR)/jtregTestThreadFactory.jar | ||
|
|
||
| JTREG_VALUE_CLASS_PLUGIN_DIR := $(TEST_IMAGE_DIR)/jtreg_value_class_plugin | ||
| JTREG_VALUE_CLASS_PLUGIN_JAR := $(JTREG_VALUE_CLASS_PLUGIN_DIR)/valueClassPlugin.jar | ||
|
|
||
| JTREG_FAILURE_HANDLER_TIMEOUT ?= 0 | ||
|
|
||
| ifneq ($(wildcard $(JTREG_FAILURE_HANDLER)), ) | ||
|
|
@@ -206,7 +209,7 @@ $(eval $(call ParseKeywordVariable, JTREG, \ | |
| SINGLE_KEYWORDS := JOBS TIMEOUT_FACTOR FAILURE_HANDLER_TIMEOUT \ | ||
| TEST_MODE ASSERT VERBOSE RETAIN TEST_THREAD_FACTORY JVMTI_STRESS_AGENT \ | ||
| MAX_MEM RUN_PROBLEM_LISTS RETRY_COUNT REPEAT_COUNT MAX_OUTPUT REPORT \ | ||
| AOT_JDK MANUAL $(CUSTOM_JTREG_SINGLE_KEYWORDS), \ | ||
| AOT_JDK MANUAL VALUE_CLASS_PLUGIN $(CUSTOM_JTREG_SINGLE_KEYWORDS), \ | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When adding a new option for testing, please add documentation describing what it is and what it does in doc/testing.[md|html]. It's pretty obvious this has to do with value classes testing, but what does using the plugin actually do? Is the option name
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This guy runs by replacing some classes used in some collection tests with a value class version. @bwhuang-us what do you think of this?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure, I will update the testing documentation.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See the following bug comment for my analysis: Summary: This issue is resolved by the fix for JDK-8385601. |
||
| STRING_KEYWORDS := OPTIONS JAVA_OPTIONS VM_OPTIONS KEYWORDS \ | ||
| EXTRA_PROBLEM_LISTS LAUNCHER_OPTIONS \ | ||
| $(CUSTOM_JTREG_STRING_KEYWORDS), \ | ||
|
|
@@ -877,6 +880,20 @@ define SetupRunJtregTestBody | |
| )) | ||
| endif | ||
|
|
||
| ifneq ($$(JTREG_VALUE_CLASS_PLUGIN), ) | ||
| ifneq ($$(wildcard $$(JTREG_VALUE_CLASS_PLUGIN_JAR)), ) | ||
| $1_JTREG_BASIC_OPTIONS += -cpa:$$(JTREG_VALUE_CLASS_PLUGIN_JAR) | ||
| endif | ||
| $1_JTREG_BASIC_OPTIONS += -vmoption:--enable-preview | ||
| $1_JTREG_BASIC_OPTIONS += -javacoption:-XDaccessInternalAPI | ||
| $1_JTREG_BASIC_OPTIONS += -javacoption:--source -javacoption:$(VERSION_FEATURE) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this really necessary? Wouldn't the current feature version be the default value for
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I think it is necessary. javac may default to the current source level normally, but once --enable-preview is present, javac requires the source level to be explicit. Otherwise it exits with: error: --enable-preview must be used with either -source or --release
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See the following bug comment for my analysis: Summary: I believe this comment is resolved. |
||
| $1_JTREG_BASIC_OPTIONS += -javacoption:--enable-preview | ||
| $1_JTREG_BASIC_OPTIONS += -javacoption:-Xplugin:ValueClassPlugin | ||
| $1_JTREG_BASIC_OPTIONS += $$(addprefix $$(JTREG_PROBLEM_LIST_PREFIX), $$(wildcard \ | ||
| $$(addprefix $$($1_TEST_ROOT)/, ProblemList-ValueClass.txt) \ | ||
| )) | ||
| endif | ||
|
|
||
| ifneq ($$(JTREG_JVMTI_STRESS_AGENT), ) | ||
| AGENT := $$(LIBRARY_PREFIX)JvmtiStressAgent$$(SHARED_LIBRARY_SUFFIX)=$$(JTREG_JVMTI_STRESS_AGENT) | ||
| $1_JTREG_BASIC_OPTIONS += -javaoption:'-agentpath:$(TEST_IMAGE_DIR)/hotspot/jtreg/native/$$(AGENT)' | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like left over project specific logic that should probably be removed before integration into the jdk repo.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See the following bug comment for my analysis:
https://bugs.openjdk.org/browse/JDK-8385743?focusedId=14884160&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14884160
This is the summary from that analysis:
@lfoltan - What is the plan for the 'lworld' branch after JEP-401 integrates?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Discussed today, we'll likely freeze
lworldat the point of integration, and do later development in other branches of thevalhallarepository. (Not sure the implications for this file, but hope that helps.)