Skip to content

8358696: Assert with extreme values for -XX:BciProfileWidth #26139

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

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
+8 −7
Open

8358696: Assert with extreme values for -XX:BciProfileWidth #26139

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
5f54ee1
initial commit
sarannat Jul 4, 2025
7606609
Modified the upper bound
sarannat Jul 4, 2025
6a36457
Fix for AArch64
sarannat Jul 21, 2025
a32b6ea
addressing review comment 1
sarannat Jul 22, 2025
c3a85cd
fixing test failures due to intx -> int of BciProfileWidth
sarannat Jul 23, 2025
2d0084b
fixing copyright
sarannat Jul 23, 2025
2fc4b0b
addressing review comment by adding intx flag
sarannat Jul 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/hotspot/share/cds/filemap.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1985,7 +1985,7 @@ bool FileMapHeader::validate() {
if (_bci_profile_width != BciProfileWidth) {
MetaspaceShared::report_loading_error("The %s's BciProfileWidth setting (%d)"
" does not equal the current BciProfileWidth setting (%d).", file_type,
(int)_bci_profile_width, (int)BciProfileWidth);
_bci_profile_width, BciProfileWidth);
return false;
}
if (_type_profile_casts != TypeProfileCasts) {
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/cds/filemap.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ class FileMapHeader: private CDSFileMapHeaderBase {
int _type_profile_args_limit;
int _type_profile_parms_limit;
intx _type_profile_width;
intx _bci_profile_width;
int _bci_profile_width;
bool _profile_traps;
bool _type_profile_casts;
int _spec_trap_limit_extra_entries;
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/jvmci/jvmciCompilerToVMInit.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,7 @@ JVMCIObjectArray CompilerToVM::initialize_intrinsics(JVMCI_TRAPS) {
do_int_flag(AllocatePrefetchLines) \
do_int_flag(AllocatePrefetchStepSize) \
do_int_flag(AllocatePrefetchStyle) \
do_intx_flag(BciProfileWidth) \
do_int_flag(BciProfileWidth) \
do_bool_flag(BootstrapJVMCI) \
do_bool_flag(CITime) \
do_bool_flag(CITimeEach) \
Expand Down
3 changes: 2 additions & 1 deletion src/hotspot/share/runtime/globals.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1351,8 +1351,9 @@ const int ObjectAlignmentInBytes = 8;
"Number of receiver types to record in call/cast profile") \
range(0, 8) \
\
develop(intx, BciProfileWidth, 2, \
develop(int, BciProfileWidth, 2, \
"Number of return bci's to record in ret profile") \
range(0, AARCH64_ONLY(1000) NOT_AARCH64(5000)) \
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not too sure of the usual number of returns but even just 1000 sounds quite big as maximum. Do you think we could use that for all architectures?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for the review. I have tested 1000 by reducing the InterpreterCodeSize to the smallest value in all the specified architecture in the source code on both AArch64 and x86. It works for 1000. Hence, I think it should work on all architectures. Do you propose I make it 1000 (or a lesser value) for all architecture ?

\
Copy link
Member

Choose a reason for hiding this comment

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

Maybe that's one empty line too much (cf. other spacing just around).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you. I fixed this.

product(intx, PerMethodRecompilationCutoff, 400, \
"After recompiling N times, stay in the interpreter (-1=>'Inf')") \
Expand Down
2 changes: 1 addition & 1 deletion src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/oops/MethodData.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ private static synchronized void initialize(TypeDataBase db) throws WrongTypeExc
if (flag.getName().equals("TypeProfileWidth")) {
TypeProfileWidth = (int)flag.getIntx();
} else if (flag.getName().equals("BciProfileWidth")) {
BciProfileWidth = (int)flag.getIntx();
BciProfileWidth = (int)flag.getInt();
} else if (flag.getName().equals("CompileThreshold")) {
CompileThreshold = (int)flag.getIntx();
}
Expand Down
4 changes: 2 additions & 2 deletions test/lib-test/jdk/test/whitebox/vm_flags/IntxTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2014, 2021, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2014, 2025, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -36,9 +36,9 @@
import jdk.test.lib.Platform;
Copy link
Member

Choose a reason for hiding this comment

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

NIt: copyright date

public class IntxTest {
private static final String FLAG_NAME = "OnStackReplacePercentage";
private static final String FLAG_DEBUG_NAME = "BciProfileWidth";
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we might want use another intx flag instead of just removing this (just to keep testing the WhiteBox)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I addressed this comment by adding BinarySwitchThreshold intx develop flag now.

private static final long COMPILE_THRESHOLD = VmFlagTest.WHITE_BOX.getIntxVMFlag("CompileThreshold");
private static final Long[] TESTS = {0L, 100L, (long)(Integer.MAX_VALUE>>3)*100L};
private static final String FLAG_DEBUG_NAME = "BinarySwitchThreshold";

public static void main(String[] args) throws Exception {
find_and_set_max_osrp();
Expand Down