Skip to content

8353175: Eliminate double iteration of stream in FieldDescriptor reinitialization #2010

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions src/hotspot/share/oops/fieldStreams.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ class FieldStreamBase : public StackObj {

// Convenient methods

FieldInfo to_FieldInfo() {
const FieldInfo& to_FieldInfo() const {
return _fi_buf;
}

Expand All @@ -131,7 +131,7 @@ class FieldStreamBase : public StackObj {
// bridge to a heavier API:
fieldDescriptor& field_descriptor() const {
fieldDescriptor& field = const_cast<fieldDescriptor&>(_fd_buf);
field.reinitialize(field_holder(), _index);
field.reinitialize(field_holder(), to_FieldInfo());
return field;
}
};
Expand Down
35 changes: 13 additions & 22 deletions src/hotspot/share/oops/instanceKlass.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1649,7 +1649,7 @@ bool InstanceKlass::find_local_field(Symbol* name, Symbol* sig, fieldDescriptor*
Symbol* f_name = fs.name();
Symbol* f_sig = fs.signature();
if (f_name == name && f_sig == sig) {
fd->reinitialize(const_cast<InstanceKlass*>(this), fs.index());
fd->reinitialize(const_cast<InstanceKlass*>(this), fs.to_FieldInfo());
return true;
}
}
Expand Down Expand Up @@ -1718,7 +1718,7 @@ Klass* InstanceKlass::find_field(Symbol* name, Symbol* sig, bool is_static, fiel
bool InstanceKlass::find_local_field_from_offset(int offset, bool is_static, fieldDescriptor* fd) const {
for (JavaFieldStream fs(this); !fs.done(); fs.next()) {
if (fs.offset() == offset) {
fd->reinitialize(const_cast<InstanceKlass*>(this), fs.index());
fd->reinitialize(const_cast<InstanceKlass*>(this), fs.to_FieldInfo());
if (fd->is_static() == is_static) return true;
}
}
Expand Down Expand Up @@ -1779,19 +1779,16 @@ void InstanceKlass::do_nonstatic_fields(FieldClosure* cl) {
if (super != nullptr) {
super->do_nonstatic_fields(cl);
}
fieldDescriptor fd;
int length = java_fields_count();
for (int i = 0; i < length; i += 1) {
fd.reinitialize(this, i);
for (JavaFieldStream fs(this); !fs.done(); fs.next()) {
fieldDescriptor& fd = fs.field_descriptor();
if (!fd.is_static()) {
cl->do_field(&fd);
}
}
}

// first in Pair is offset, second is index.
static int compare_fields_by_offset(Pair<int,int>* a, Pair<int,int>* b) {
return a->first - b->first;
static int compare_fields_by_offset(FieldInfo* a, FieldInfo* b) {
return a->offset() - b->offset();
}

void InstanceKlass::print_nonstatic_fields(FieldClosure* cl) {
Expand All @@ -1800,26 +1797,20 @@ void InstanceKlass::print_nonstatic_fields(FieldClosure* cl) {
super->print_nonstatic_fields(cl);
}
ResourceMark rm;
fieldDescriptor fd;
// In DebugInfo nonstatic fields are sorted by offset.
GrowableArray<Pair<int,int> > fields_sorted;
int i = 0;
GrowableArray<FieldInfo> fields_sorted;
for (AllFieldStream fs(this); !fs.done(); fs.next()) {
if (!fs.access_flags().is_static()) {
fd = fs.field_descriptor();
Pair<int,int> f(fs.offset(), fs.index());
fields_sorted.push(f);
i++;
fields_sorted.push(fs.to_FieldInfo());
}
}
if (i > 0) {
int length = i;
assert(length == fields_sorted.length(), "duh");
// _sort_Fn is defined in growableArray.hpp.
int length = fields_sorted.length();
if (length > 0) {
fields_sorted.sort(compare_fields_by_offset);
fieldDescriptor fd;
for (int i = 0; i < length; i++) {
fd.reinitialize(this, fields_sorted.at(i).second);
assert(!fd.is_static() && fd.offset() == fields_sorted.at(i).first, "only nonstatic fields");
fd.reinitialize(this, fields_sorted.at(i));
assert(!fd.is_static() && fd.offset() == checked_cast<int>(fields_sorted.at(i).offset()), "only nonstatic fields");
cl->do_field(&fd);
}
}
Expand Down
1 change: 1 addition & 0 deletions src/hotspot/share/oops/instanceKlass.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ class InstanceKlass: public Klass {
friend class JVMCIVMStructs;
friend class ClassFileParser;
friend class CompileReplay;
friend class FieldStream;

public:
static const KlassKind Kind = InstanceKlassKind;
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/prims/jvm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1787,7 +1787,7 @@ JVM_ENTRY(jobjectArray, JVM_GetClassDeclaredFields(JNIEnv *env, jclass ofClass,
fieldDescriptor fd;
for (JavaFieldStream fs(k); !fs.done(); fs.next()) {
if (!publicOnly || fs.access_flags().is_public()) {
fd.reinitialize(k, fs.index());
fd.reinitialize(k, fs.to_FieldInfo());
oop field = Reflection::new_field(&fd, CHECK_NULL);
result->obj_at_put(out_idx, field);
++out_idx;
Expand Down
5 changes: 2 additions & 3 deletions src/hotspot/share/runtime/fieldDescriptor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -87,16 +87,15 @@ oop fieldDescriptor::string_initial_value(TRAPS) const {
return constants()->uncached_string_at(initial_value_index(), THREAD);
}

void fieldDescriptor::reinitialize(InstanceKlass* ik, int index) {
void fieldDescriptor::reinitialize(InstanceKlass* ik, const FieldInfo& fieldinfo) {
if (_cp.is_null() || field_holder() != ik) {
_cp = constantPoolHandle(Thread::current(), ik->constants());
// _cp should now reference ik's constant pool; i.e., ik is now field_holder.
// If the class is a scratch class, the constant pool points to the original class,
// but that's ok because of constant pool merging.
assert(field_holder() == ik || ik->is_scratch_class(), "must be already initialized to this class");
}
_fieldinfo= ik->field(index);
assert((int)_fieldinfo.index() == index, "just checking");
_fieldinfo = fieldinfo;
guarantee(_fieldinfo.name_index() != 0 && _fieldinfo.signature_index() != 0, "bad constant pool index for fieldDescriptor");
}

Expand Down
4 changes: 2 additions & 2 deletions src/hotspot/share/runtime/fieldDescriptor.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ class fieldDescriptor {
public:
fieldDescriptor() {}
fieldDescriptor(InstanceKlass* ik, int index) {
reinitialize(ik, index);
reinitialize(ik, ik->field(index));
}
inline Symbol* name() const;
inline Symbol* signature() const;
Expand Down Expand Up @@ -102,7 +102,7 @@ class fieldDescriptor {
inline void set_has_initialized_final_update(const bool value);

// Initialization
void reinitialize(InstanceKlass* ik, int index);
void reinitialize(InstanceKlass* ik, const FieldInfo& fieldinfo);

// Print
void print() const;
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/runtime/reflectionUtils.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ class FieldStream : public KlassStream {
// bridge to a heavier API:
fieldDescriptor& field_descriptor() const {
fieldDescriptor& field = const_cast<fieldDescriptor&>(_fd_buf);
field.reinitialize(_klass, _index);
field.reinitialize(_klass, _klass->field(_index));
return field;
}
};
Expand Down