Skip to content

Commit c243dfd

Browse files
HalidOdatHalidOdat
committed
PR comment: Add documentation and de-duplicate instruction implementations
1 parent 65b14dc commit c243dfd

File tree

2 files changed

+92
-82
lines changed

2 files changed

+92
-82
lines changed

‎core/engine/src/vm/opcode/get/property.rs‎

Lines changed: 91 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -7,21 +7,7 @@ use crate::{
77
vm::opcode::{Operation, VaryingOperand},
88
};
99

10-
fn js_string_get(this: &JsValue, key: &PropertyKey) -> Option<JsValue> {
11-
let this = this.as_string()?;
12-
match key {
13-
PropertyKey::String(name) if *name == StaticJsStrings::LENGTH => Some(this.len().into()),
14-
PropertyKey::Index(index) => Some(
15-
this.get(index.get() as usize)
16-
.map_or_else(JsValue::undefined, |char| {
17-
js_string!([char].as_slice()).into()
18-
}),
19-
),
20-
_ => None,
21-
}
22-
}
23-
24-
pub(crate) fn get_by_name<const LENGTH: bool>(
10+
fn get_by_name<const LENGTH: bool>(
2511
(dst, object, receiver, index): (VaryingOperand, &JsValue, &JsValue, VaryingOperand),
2612
context: &mut Context,
2713
) -> JsResult<()> {
@@ -33,13 +19,22 @@ pub(crate) fn get_by_name<const LENGTH: bool>(
3319
context.vm.set_register(dst.into(), value);
3420
return Ok(());
3521
} else if let Some(string) = object.as_string() {
22+
// NOTE: Since we’re using the prototype returned directly by `base_class()`,
23+
// we need to handle string primitives separately due to the
24+
// string exotic internal methods.
3625
context
3726
.vm
3827
.set_register(dst.into(), (string.len() as u32).into());
3928
return Ok(());
4029
}
4130
}
4231

32+
// OPTIMIZATION:
33+
// Instead of calling `to_object()`, which creates a temporary wrapper object for primitive
34+
// values (e.g., numbers, strings, booleans) just to query their prototype chain.
35+
//
36+
// To prevent the creation of a temporary JsObject, we directly retrieve the prototype that
37+
// `to_object()` would produce, such as `Number.prototype`, `String.prototype`, etc.
4338
let object = object.base_class(context)?;
4439

4540
let ic = &context.vm.frame().code_block().ic[usize::from(index)];
@@ -74,7 +69,7 @@ pub(crate) fn get_by_name<const LENGTH: bool>(
7469

7570
// Cache the property.
7671
let slot = *context.slot();
77-
if slot.is_cachable() {
72+
if slot.is_cacheable() {
7873
let ic = &context.vm.frame().code_block.ic[usize::from(index)];
7974
let object_borrowed = object.borrow();
8075
let shape = object_borrowed.shape();
@@ -85,6 +80,82 @@ pub(crate) fn get_by_name<const LENGTH: bool>(
8580
Ok(())
8681
}
8782

83+
fn get_by_value<const PUSH_KEY: bool>(
84+
(dst, key, receiver, object): (
85+
VaryingOperand,
86+
VaryingOperand,
87+
VaryingOperand,
88+
VaryingOperand,
89+
),
90+
context: &mut Context,
91+
) -> JsResult<()> {
92+
let key_value = context.vm.get_register(key.into()).clone();
93+
let base = context.vm.get_register(object.into()).clone();
94+
let object = base.base_class(context)?;
95+
let key_value = key_value.to_property_key(context)?;
96+
97+
// Fast Path
98+
//
99+
// NOTE: Since we’re using the prototype returned directly by `base_class()`,
100+
// we need to handle string primitives separately due to the
101+
// string exotic internal methods.
102+
match &key_value {
103+
PropertyKey::Index(index) => {
104+
if object.is_array() {
105+
let object_borrowed = object.borrow();
106+
if let Some(element) = object_borrowed.properties().get_dense_property(index.get())
107+
{
108+
if PUSH_KEY {
109+
context.vm.set_register(key.into(), key_value.into());
110+
}
111+
112+
context.vm.set_register(dst.into(), element);
113+
return Ok(());
114+
}
115+
} else if let Some(string) = base.as_string() {
116+
let value = string
117+
.get(index.get() as usize)
118+
.map_or_else(JsValue::undefined, |char| {
119+
js_string!([char].as_slice()).into()
120+
});
121+
122+
if PUSH_KEY {
123+
context.vm.set_register(key.into(), key_value.into());
124+
}
125+
context.vm.set_register(dst.into(), value);
126+
return Ok(());
127+
}
128+
}
129+
PropertyKey::String(string) if *string == StaticJsStrings::LENGTH => {
130+
if let Some(string) = base.as_string() {
131+
let value = string.len().into();
132+
133+
if PUSH_KEY {
134+
context.vm.set_register(key.into(), key_value.into());
135+
}
136+
context.vm.set_register(dst.into(), value);
137+
return Ok(());
138+
}
139+
}
140+
_ => {}
141+
}
142+
143+
let receiver = context.vm.get_register(receiver.into());
144+
145+
// Slow path:
146+
let result = object.__get__(
147+
&key_value,
148+
receiver.clone(),
149+
&mut InternalMethodPropertyContext::new(context),
150+
)?;
151+
152+
if PUSH_KEY {
153+
context.vm.set_register(key.into(), key_value.into());
154+
}
155+
context.vm.set_register(dst.into(), result);
156+
Ok(())
157+
}
158+
88159
#[derive(Debug, Clone, Copy)]
89160
pub(crate) struct GetLengthProperty;
90161

@@ -169,44 +240,15 @@ pub(crate) struct GetPropertyByValue;
169240
impl GetPropertyByValue {
170241
#[inline(always)]
171242
pub(crate) fn operation(
172-
(dst, key, receiver, object): (
243+
args: (
173244
VaryingOperand,
174245
VaryingOperand,
175246
VaryingOperand,
176247
VaryingOperand,
177248
),
178249
context: &mut Context,
179250
) -> JsResult<()> {
180-
let key = context.vm.get_register(key.into()).clone();
181-
let base = context.vm.get_register(object.into()).clone();
182-
let object = base.base_class(context)?;
183-
let key = key.to_property_key(context)?;
184-
185-
// Fast Path
186-
if object.is_array()
187-
&& let PropertyKey::Index(index) = &key
188-
{
189-
let object_borrowed = object.borrow();
190-
if let Some(element) = object_borrowed.properties().get_dense_property(index.get()) {
191-
context.vm.set_register(dst.into(), element);
192-
return Ok(());
193-
}
194-
} else if let Some(value) = js_string_get(&base, &key) {
195-
context.vm.set_register(dst.into(), value);
196-
return Ok(());
197-
}
198-
199-
let receiver = context.vm.get_register(receiver.into());
200-
201-
// Slow path:
202-
let result = object.__get__(
203-
&key,
204-
receiver.clone(),
205-
&mut InternalMethodPropertyContext::new(context),
206-
)?;
207-
208-
context.vm.set_register(dst.into(), result);
209-
Ok(())
251+
get_by_value::<false>(args, context)
210252
}
211253
}
212254

@@ -226,47 +268,15 @@ pub(crate) struct GetPropertyByValuePush;
226268
impl GetPropertyByValuePush {
227269
#[inline(always)]
228270
pub(crate) fn operation(
229-
(dst, key, receiver, object): (
271+
args: (
230272
VaryingOperand,
231273
VaryingOperand,
232274
VaryingOperand,
233275
VaryingOperand,
234276
),
235277
context: &mut Context,
236278
) -> JsResult<()> {
237-
let key_value = context.vm.get_register(key.into()).clone();
238-
let base = context.vm.get_register(object.into()).clone();
239-
let object = base.base_class(context)?;
240-
let key_value = key_value.to_property_key(context)?;
241-
242-
// Fast Path
243-
if object.is_array()
244-
&& let PropertyKey::Index(index) = &key_value
245-
{
246-
let object_borrowed = object.borrow();
247-
if let Some(element) = object_borrowed.properties().get_dense_property(index.get()) {
248-
context.vm.set_register(key.into(), key_value.into());
249-
context.vm.set_register(dst.into(), element);
250-
return Ok(());
251-
}
252-
} else if let Some(value) = js_string_get(&base, &key_value) {
253-
context.vm.set_register(key.into(), key_value.into());
254-
context.vm.set_register(dst.into(), value);
255-
return Ok(());
256-
}
257-
258-
let receiver = context.vm.get_register(receiver.into());
259-
260-
// Slow path:
261-
let result = object.__get__(
262-
&key_value,
263-
receiver.clone(),
264-
&mut InternalMethodPropertyContext::new(context),
265-
)?;
266-
267-
context.vm.set_register(key.into(), key_value.into());
268-
context.vm.set_register(dst.into(), result);
269-
Ok(())
279+
get_by_value::<true>(args, context)
270280
}
271281
}
272282

‎core/engine/src/vm/opcode/set/property.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ fn set_by_name(
7171

7272
// Cache the property.
7373
let slot = *context.slot();
74-
if succeeded && slot.is_cachable() {
74+
if succeeded && slot.is_cacheable() {
7575
let ic = &context.vm.frame().code_block.ic[usize::from(index)];
7676
let object_borrowed = object.borrow();
7777
let shape = object_borrowed.shape();

0 commit comments

Comments
 (0)