Skip to content

Commit

Permalink
Refactoring: remove descriptor from string field representation
Browse files Browse the repository at this point in the history
  • Loading branch information
@hextriclosan
hextriclosan committed Nov 29, 2024
1 parent 6c79084 commit e45b3bd
Show file tree
Hide file tree
Showing 6 changed files with 26 additions and 40 deletions.
36 changes: 13 additions & 23 deletions vm/src/execution_engine/ops_reference_processor.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,7 @@ pub(crate) fn process(
match code {
GETSTATIC => {
let stack_frame = stack_frames.last_mut().unwrap();
let (class_name, field_name, _field_descriptor) =
get_field_info(stack_frame, current_class_name)?;
let (class_name, field_name) = get_field_info(stack_frame, current_class_name)?;

let field = with_method_area(|method_area| {
method_area.lookup_for_static_field(&class_name, &field_name)
Expand All @@ -40,8 +39,7 @@ pub(crate) fn process(
}
PUTSTATIC => {
let stack_frame = stack_frames.last_mut().unwrap();
let (class_name, field_name, _field_descriptor) =
get_field_info(stack_frame, current_class_name)?;
let (class_name, field_name) = get_field_info(stack_frame, current_class_name)?;

let (len, field_ref) = {
let field_ref = with_method_area(|method_area| {
Expand All @@ -63,34 +61,26 @@ pub(crate) fn process(
}
GETFIELD => {
let stack_frame = stack_frames.last_mut().unwrap();
let (class_name, field_name, field_descriptor) =
get_field_info(stack_frame, current_class_name)?;
let field_name_type = format!("{field_name}:{field_descriptor}");
let (class_name, field_name) = get_field_info(stack_frame, current_class_name)?;
let objectref = stack_frame.pop();
let value = with_heap_read_lock(|heap| {
heap.get_object_field_value(
objectref,
class_name.as_str(),
field_name_type.as_str(),
)
heap.get_object_field_value(objectref, class_name.as_str(), field_name.as_str())
})?;

value.iter().rev().for_each(|x| stack_frame.push(*x));

stack_frame.incr_pc();
trace!("GETFIELD -> objectref={objectref}, class_name={class_name}, field_name_type={field_name_type}, value={value:?}");
trace!("GETFIELD -> objectref={objectref}, class_name={class_name}, field_name={field_name}, value={value:?}");
}
PUTFIELD => {
let stack_frame = stack_frames.last_mut().unwrap();
let (class_name, field_name, field_descriptor) =
get_field_info(stack_frame, current_class_name)?;
let field_name_type = format!("{field_name}:{field_descriptor}");
let (class_name, field_name) = get_field_info(stack_frame, current_class_name)?;
let type_descriptor = with_method_area(|method_area| {
method_area
.lookup_for_field_descriptor(&class_name, &field_name_type)
.lookup_for_field_descriptor(&class_name, &field_name)
.ok_or_else(|| {
Error::new_constant_pool(&format!(
"Error getting type descriptor for {class_name}.{field_name_type}"
"Error getting type descriptor for {class_name}.{field_name}"
))
})
})?;
Expand All @@ -107,12 +97,12 @@ pub(crate) fn process(
heap.set_object_field_value(
objectref,
class_name.as_str(),
field_name_type.as_str(),
field_name.as_str(),
value.clone(),
)
})?;

trace!("PUTFIELD -> objectref={objectref}, class_name={class_name}, field_name_type={field_name_type} value={value:?}");
trace!("PUTFIELD -> objectref={objectref}, class_name={class_name}, field_name={field_name} value={value:?}");
stack_frame.incr_pc();
}
INVOKEVIRTUAL => {
Expand Down Expand Up @@ -497,18 +487,18 @@ fn frame(stack_frames: &mut [StackFrame]) -> &mut StackFrame {
fn get_field_info(
stack_frame: &mut StackFrame,
current_class_name: &str,
) -> crate::error::Result<(String, String, String)> {
) -> crate::error::Result<(String, String)> {
let fieldref_constpool_index = stack_frame.extract_two_bytes() as u16;

let rc = with_method_area(|method_area| method_area.get(current_class_name))?;
let cpool_helper = rc.cpool_helper();

let (class_name, field_name, field_descriptor) = cpool_helper
let (class_name, field_name) = cpool_helper
.get_full_field_info(fieldref_constpool_index)
.ok_or_else(|| {
Error::new_constant_pool(&format!(
"Error getting full field info by index {fieldref_constpool_index}"
))
})?;
Ok((class_name, field_name, field_descriptor))
Ok((class_name, field_name))
}
2 changes: 1 addition & 1 deletion vm/src/execution_engine/reflection_class_loader.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ impl ReflectionClassLoader {
})?;
reflection_instance.set_field_value(
"java/lang/Class",
"componentType:Ljava/lang/Class;",
"componentType",
vec![component_type_ref],
)?;

Expand Down
12 changes: 4 additions & 8 deletions vm/src/method_area/cpool_helper.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ impl CPoolHelper {
}
}

pub fn get_full_field_info(&self, index: u16) -> Option<(String, String, String)> {
pub fn get_full_field_info(&self, index: u16) -> Option<(String, String)> {
let (class_index, name_and_type_index) = match self.get(CPoolType::Fieldref, index)? {
ConstantPool::Fieldref {
class_index,
Expand All @@ -142,9 +142,9 @@ impl CPoolHelper {
}?;

let class_name = self.get_class_name(*class_index)?;
let (field_name, field_descriptor) = self.get_name_and_type(*name_and_type_index)?;
let (field_name, _) = self.get_name_and_type(*name_and_type_index)?;

Some((class_name, field_name, field_descriptor))
Some((class_name, field_name))
}

pub fn get_full_method_info(&self, index: u16) -> Option<(String, String, String)> {
Expand Down Expand Up @@ -431,11 +431,7 @@ mod tests {

let actual = resolver.get_full_field_info(3);
assert_eq!(
Some((
"TheClass".to_string(),
"theField".to_string(),
"I".to_string()
)),
Some(("TheClass".to_string(), "theField".to_string(),)),
actual
);
}
Expand Down
3 changes: 2 additions & 1 deletion vm/src/method_area/java_class.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,8 @@ impl JavaClass {
&self.this_class_name,
&mut instance_fields_hierarchy,
)
}).expect("error getting instance fields hierarchy");
})
.expect("error getting instance fields hierarchy");

instance_fields_hierarchy
}))
Expand Down
9 changes: 4 additions & 5 deletions vm/src/method_area/method_area.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -267,8 +267,7 @@ impl MethodArea {
if field_info.access_flags().contains(FieldFlags::ACC_STATIC) {
static_field_by_name.insert(field_name, Arc::new(Field::new(descriptor)));
} else {
let field_name_key = format!("{field_name}:{field_descriptor}");
non_static_field_descriptors.insert(field_name_key, descriptor);
non_static_field_descriptors.insert(field_name, descriptor);
}
}

Expand Down Expand Up @@ -357,16 +356,16 @@ impl MethodArea {
pub fn lookup_for_field_descriptor(
&self,
class_name: &str,
field_name_type: &str,
field_name: &str,
) -> Option<TypeDescriptor> {
let rc = self.get(class_name).ok()?;

if let Some(type_descriptor) = rc.instance_field_descriptor(field_name_type) {
if let Some(type_descriptor) = rc.instance_field_descriptor(field_name) {
Some(type_descriptor.clone())
} else {
let parent_class_name = rc.parent().clone()?;

self.lookup_for_field_descriptor(&parent_class_name, field_name_type)
self.lookup_for_field_descriptor(&parent_class_name, field_name)
}
}

Expand Down
4 changes: 2 additions & 2 deletions vm/src/system_native/string.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ use crate::execution_engine::string_pool_helper::StringPoolHelper;
use crate::heap::heap::with_heap_read_lock;

const STRING_CLASS_NAME: &str = "java/lang/String";
const VALUE_FIELD: &str = "value:[B";
const CODER_FIELD: &str = "coder:B";
const VALUE_FIELD: &str = "value";
const CODER_FIELD: &str = "coder";

pub(crate) fn get_utf8_string_by_ref(string_ref: i32) -> crate::error::Result<String> {
let array_ref = with_heap_read_lock(|heap| {
Expand Down

0 comments on commit e45b3bd

Please sign in to comment.