Skip to content

Commit

Permalink
8298475: Remove JVM_ACC_PROMOTED_FLAGS
Browse files Browse the repository at this point in the history
Reviewed-by: sspitsyn, dcubed, dholmes
  • Loading branch information
coleenp committed Dec 14, 2022
1 parent 9ee5037 commit 2e801e1
Show file tree
Hide file tree
Showing 10 changed files with 41 additions and 42 deletions.
30 changes: 13 additions & 17 deletions src/hotspot/share/classfile/classFileParser.cpp
Expand Up @@ -2260,18 +2260,16 @@ void ClassFileParser::copy_method_annotations(ConstMethod* cm,
// Method* to save footprint, so we only know the size of the resulting Method* when the
// entire method attribute is parsed.
//
// The promoted_flags parameter is used to pass relevant access_flags
// from the method back up to the containing klass. These flag values
// are added to klass's access_flags.
// The has_localvariable_table parameter is used to pass up the value to InstanceKlass.

Method* ClassFileParser::parse_method(const ClassFileStream* const cfs,
bool is_interface,
const ConstantPool* cp,
AccessFlags* const promoted_flags,
bool* const has_localvariable_table,
TRAPS) {
assert(cfs != NULL, "invariant");
assert(cp != NULL, "invariant");
assert(promoted_flags != NULL, "invariant");
assert(has_localvariable_table != NULL, "invariant");

ResourceMark rm(THREAD);
// Parse fixed parts:
Expand Down Expand Up @@ -2820,7 +2818,7 @@ Method* ClassFileParser::parse_method(const ClassFileStream* const cfs,

// Copy class file LVT's/LVTT's into the HotSpot internal LVT.
if (total_lvt_length > 0) {
promoted_flags->set_has_localvariable_table();
*has_localvariable_table = true;
copy_localvariable_table(m->constMethod(),
lvt_cnt,
localvariable_table_length,
Expand Down Expand Up @@ -2876,18 +2874,15 @@ Method* ClassFileParser::parse_method(const ClassFileStream* const cfs,
}


// The promoted_flags parameter is used to pass relevant access_flags
// from the methods back up to the containing klass. These flag values
// are added to klass's access_flags.
// Side-effects: populates the _methods field in the parser
void ClassFileParser::parse_methods(const ClassFileStream* const cfs,
bool is_interface,
AccessFlags* promoted_flags,
bool* const has_localvariable_table,
bool* has_final_method,
bool* declares_nonstatic_concrete_methods,
TRAPS) {
assert(cfs != NULL, "invariant");
assert(promoted_flags != NULL, "invariant");
assert(has_localvariable_table != NULL, "invariant");
assert(has_final_method != NULL, "invariant");
assert(declares_nonstatic_concrete_methods != NULL, "invariant");

Expand All @@ -2907,7 +2902,7 @@ void ClassFileParser::parse_methods(const ClassFileStream* const cfs,
Method* method = parse_method(cfs,
is_interface,
_cp,
promoted_flags,
has_localvariable_table,
CHECK);

if (method->is_final()) {
Expand Down Expand Up @@ -5333,6 +5328,10 @@ void ClassFileParser::fill_instance_klass(InstanceKlass* ik,
assert(NULL == _record_components, "invariant");
assert(NULL == _permitted_subclasses, "invariant");

if (_has_localvariable_table) {
ik->set_has_localvariable_table(true);
}

if (_has_final_method) {
ik->set_has_final_method();
}
Expand Down Expand Up @@ -5596,6 +5595,7 @@ ClassFileParser::ClassFileParser(ClassFileStream* stream,
_relax_verify(false),
_has_nonstatic_concrete_methods(false),
_declares_nonstatic_concrete_methods(false),
_has_localvariable_table(false),
_has_final_method(false),
_has_contended_fields(false),
_has_finalizer(false),
Expand Down Expand Up @@ -5901,19 +5901,15 @@ void ClassFileParser::parse_stream(const ClassFileStream* const stream,
assert(_fields != NULL, "invariant");

// Methods
AccessFlags promoted_flags;
parse_methods(stream,
_access_flags.is_interface(),
&promoted_flags,
&_has_localvariable_table,
&_has_final_method,
&_declares_nonstatic_concrete_methods,
CHECK);

assert(_methods != NULL, "invariant");

// promote flags from parse_methods() to the klass' flags
_access_flags.add_promoted_flags(promoted_flags.as_int());

if (_declares_nonstatic_concrete_methods) {
_has_nonstatic_concrete_methods = true;
}
Expand Down
5 changes: 3 additions & 2 deletions src/hotspot/share/classfile/classFileParser.hpp
Expand Up @@ -187,6 +187,7 @@ class ClassFileParser {

bool _has_nonstatic_concrete_methods;
bool _declares_nonstatic_concrete_methods;
bool _has_localvariable_table;
bool _has_final_method;
bool _has_contended_fields;

Expand Down Expand Up @@ -267,12 +268,12 @@ class ClassFileParser {
Method* parse_method(const ClassFileStream* const cfs,
bool is_interface,
const ConstantPool* cp,
AccessFlags* const promoted_flags,
bool* const has_localvariable_table,
TRAPS);

void parse_methods(const ClassFileStream* const cfs,
bool is_interface,
AccessFlags* const promoted_flags,
bool* const has_localvariable_table,
bool* const has_final_method,
bool* const declares_nonstatic_concrete_methods,
TRAPS);
Expand Down
3 changes: 3 additions & 0 deletions src/hotspot/share/oops/instanceKlass.hpp
Expand Up @@ -336,6 +336,9 @@ class InstanceKlass: public Klass {
bool has_nonstatic_fields() const { return _misc_status.has_nonstatic_fields(); }
void set_has_nonstatic_fields(bool b) { _misc_status.set_has_nonstatic_fields(b); }

bool has_localvariable_table() const { return _misc_status.has_localvariable_table(); }
void set_has_localvariable_table(bool b) { _misc_status.set_has_localvariable_table(b); }

// field sizes
int nonstatic_field_size() const { return _nonstatic_field_size; }
void set_nonstatic_field_size(int size) { _nonstatic_field_size = size; }
Expand Down
10 changes: 10 additions & 0 deletions src/hotspot/share/oops/instanceKlassMiscStatus.cpp
Expand Up @@ -26,6 +26,7 @@
#include "classfile/classLoader.hpp"
#include "classfile/classLoaderData.inline.hpp"
#include "oops/instanceKlassMiscStatus.hpp"
#include "runtime/safepoint.hpp"
#include "utilities/macros.hpp"

#if INCLUDE_CDS
Expand Down Expand Up @@ -57,4 +58,13 @@ void InstanceKlassMiscStatus::assign_class_loader_type(const ClassLoaderData* cl
set_shared_class_loader_type(ClassLoader::APP_LOADER);
}
}

#ifdef ASSERT
void InstanceKlassMiscStatus::assert_is_safe(bool set) {
// Setting a flag is safe if it's set once or at a safepoint. RedefineClasses can set or
// reset flags at a safepoint.
assert(!set || SafepointSynchronize::is_at_safepoint(), "set once or at safepoint");
}
#endif // ASSERT

#endif // INCLUDE_CDS
6 changes: 4 additions & 2 deletions src/hotspot/share/oops/instanceKlassMiscStatus.hpp
Expand Up @@ -45,7 +45,8 @@ class InstanceKlassMiscStatus {
flag(is_shared_boot_class , 1 << 10) /* defining class loader is boot class loader */ \
flag(is_shared_platform_class , 1 << 11) /* defining class loader is platform class loader */ \
flag(is_shared_app_class , 1 << 12) /* defining class loader is app class loader */ \
flag(has_contended_annotations , 1 << 13) /* has @Contended annotation */
flag(has_contended_annotations , 1 << 13) /* has @Contended annotation */ \
flag(has_localvariable_table , 1 << 14) /* has localvariable information */

#define IK_FLAGS_ENUM_NAME(name, value) _misc_##name = value,
enum {
Expand All @@ -72,7 +73,7 @@ class InstanceKlassMiscStatus {

#define IK_FLAGS_SET(name, ignore) \
void set_##name(bool b) { \
assert(!name(), "set once"); \
assert_is_safe(name()); \
if (b) _flags |= _misc_##name; \
}
IK_FLAGS_DO(IK_FLAGS_SET)
Expand All @@ -85,6 +86,7 @@ class InstanceKlassMiscStatus {
void set_shared_class_loader_type(s2 loader_type);

void assign_class_loader_type(const ClassLoaderData* cld);
void assert_is_safe(bool set) NOT_DEBUG_RETURN;
};

#endif // SHARE_OOPS_INSTANCEKLASSMISCSTATUS_HPP
2 changes: 1 addition & 1 deletion src/hotspot/share/prims/jvmtiEnv.cpp
Expand Up @@ -3522,7 +3522,7 @@ JvmtiEnv::GetLocalVariableTable(Method* method, jint* entry_count_ptr, jvmtiLoca

// does the klass have any local variable information?
InstanceKlass* ik = method->method_holder();
if (!ik->access_flags().has_localvariable_table()) {
if (!ik->has_localvariable_table()) {
return (JVMTI_ERROR_ABSENT_INFORMATION);
}

Expand Down
13 changes: 3 additions & 10 deletions src/hotspot/share/prims/jvmtiRedefineClasses.cpp
Expand Up @@ -4371,16 +4371,9 @@ void VM_RedefineClasses::redefine_single_class(Thread* current, jclass the_jclas
(int)strlen(scratch_class->source_debug_extension()));

// Use of javac -g could be different in the old and the new
if (scratch_class->access_flags().has_localvariable_table() !=
the_class->access_flags().has_localvariable_table()) {

AccessFlags flags = the_class->access_flags();
if (scratch_class->access_flags().has_localvariable_table()) {
flags.set_has_localvariable_table();
} else {
flags.clear_has_localvariable_table();
}
the_class->set_access_flags(flags);
if (scratch_class->has_localvariable_table() !=
the_class->has_localvariable_table()) {
the_class->set_has_localvariable_table(scratch_class->has_localvariable_table());
}

swap_annotations(the_class, scratch_class);
Expand Down
1 change: 0 additions & 1 deletion src/hotspot/share/runtime/vmStructs.cpp
Expand Up @@ -2098,7 +2098,6 @@
declare_constant(JVM_ACC_HAS_FINALIZER) \
declare_constant(JVM_ACC_IS_CLONEABLE_FAST) \
declare_constant(JVM_ACC_HAS_LOCAL_VARIABLE_TABLE) \
declare_constant(JVM_ACC_PROMOTED_FLAGS) \
declare_constant(JVM_ACC_FIELD_ACCESS_WATCHED) \
declare_constant(JVM_ACC_FIELD_MODIFICATION_WATCHED) \
declare_constant(JVM_ACC_FIELD_INTERNAL) \
Expand Down
9 changes: 3 additions & 6 deletions src/hotspot/share/utilities/accessFlags.hpp
@@ -1,5 +1,5 @@
/*
* Copyright (c) 1997, 2021, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1997, 2022, 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 @@ -72,11 +72,9 @@ enum {
JVM_ACC_IS_BEING_REDEFINED = 0x00100000, // True if the klass is being redefined.
JVM_ACC_HAS_RESOLVED_METHODS = 0x00200000, // True if the klass has resolved methods

// Klass* and Method* flags
// Method* flags
JVM_ACC_HAS_LOCAL_VARIABLE_TABLE= 0x00400000,

JVM_ACC_PROMOTED_FLAGS = 0x00400000, // flags promoted from methods to the holding klass

// field flags
// Note: these flags must be defined in the low order 16 bits because
// InstanceKlass only stores a ushort worth of information from the
Expand Down Expand Up @@ -156,7 +154,7 @@ class AccessFlags {
bool is_hidden_class () const { return (_flags & JVM_ACC_IS_HIDDEN_CLASS ) != 0; }
bool is_value_based_class () const { return (_flags & JVM_ACC_IS_VALUE_BASED_CLASS ) != 0; }

// Klass* and Method* flags
// Method* flags
bool has_localvariable_table () const { return (_flags & JVM_ACC_HAS_LOCAL_VARIABLE_TABLE) != 0; }
void set_has_localvariable_table() { atomic_set_bits(JVM_ACC_HAS_LOCAL_VARIABLE_TABLE); }
void clear_has_localvariable_table() { atomic_clear_bits(JVM_ACC_HAS_LOCAL_VARIABLE_TABLE); }
Expand Down Expand Up @@ -184,7 +182,6 @@ class AccessFlags {
jint get_flags () const { return (_flags & JVM_ACC_WRITTEN_FLAGS); }

// Initialization
void add_promoted_flags(jint flags) { _flags |= (flags & JVM_ACC_PROMOTED_FLAGS); }
void set_field_flags(jint flags) {
assert((flags & JVM_ACC_FIELD_FLAGS) == flags, "only recognized flags");
_flags = (flags & JVM_ACC_FIELD_FLAGS);
Expand Down
Expand Up @@ -134,10 +134,8 @@ public interface ClassConstants
// True if klass supports the Clonable interface
public static final long JVM_ACC_IS_CLONEABLE = 0x80000000;

// Klass* and Method* flags
// Method* flags
public static final long JVM_ACC_HAS_LOCAL_VARIABLE_TABLE = 0x00200000;
// flags promoted from methods to the holding klass
public static final long JVM_ACC_PROMOTED_FLAGS = 0x00200000;

// field flags
// Note: these flags must be defined in the low order 16 bits because
Expand Down

1 comment on commit 2e801e1

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

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

Please sign in to comment.