Skip to content

perf(bytecompiler): avoid redundant Constant clone in IC initialization#5299

Open
tkshsbcue wants to merge 2 commits intoboa-dev:mainfrom
tkshsbcue:perf/avoid-redundant-constant-clone-in-ic-init
Open

perf(bytecompiler): avoid redundant Constant clone in IC initialization#5299
tkshsbcue wants to merge 2 commits intoboa-dev:mainfrom
tkshsbcue:perf/avoid-redundant-constant-clone-in-ic-init

Conversation

@tkshsbcue
Copy link
Copy Markdown
Contributor

@tkshsbcue tkshsbcue commented Apr 5, 2026

perf(bytecompiler): avoid redundant Constant clone in IC initialization

Description

emit_get_property_by_name and emit_set_property_by_name were cloning the entire Constant enum to extract the inner JsString, which was then cloned again for the InlineCache. This meant two JsString clones per property access site during compilation when only one is needed.

The fix borrows the constant by reference and clones only the JsString once.

Before

let Constant::String(ref name) = self.constants[name_index as usize].clone() else {
    unreachable!("there should be a string at index")
};
self.ic.push(InlineCache::new(name.clone()));
  1. .clone() on the Constant enum clones the inner JsString
  2. name.clone() clones the JsString again for the InlineCache

After

let Constant::String(name) = &self.constants[name_index as usize] else {
    unreachable!("there should be a string at index")
};
let name = name.clone();
self.ic.push(InlineCache::new(name));
  1. Borrow by reference — no clone
  2. Single JsString clone, moved directly into the InlineCache

emit_get_property_by_name and emit_set_property_by_name were cloning
the entire Constant enum just to extract the inner JsString, which was
then cloned again for the InlineCache. Borrow the constant by reference
instead and clone only the JsString once.
@tkshsbcue tkshsbcue requested a review from a team as a code owner April 5, 2026 05:23
@github-actions github-actions bot added Waiting On Review Waiting on reviews from the maintainers C-VM Issues and PRs related to the Boa Virtual Machine. and removed Waiting On Review Waiting on reviews from the maintainers labels Apr 5, 2026
@github-actions github-actions bot added this to the v1.0.0 milestone Apr 5, 2026
@github-actions github-actions bot added the Waiting On Review Waiting on reviews from the maintainers label Apr 5, 2026
@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 5, 2026

Test262 conformance changes

Test result main count PR count difference
Total 53,125 53,125 0
Passed 50,827 50,827 0
Ignored 1,482 1,482 0
Failed 816 816 0
Panics 0 0 0
Conformance 95.67% 95.67% 0.00%

Tested main commit: f6021b3d3dd1249c9569be77f0ca9ccf00ea318a
Tested PR commit: 832efafc0ff6da7e9ee6433d264c03a8613db114
Compare commits: f6021b3...832efaf

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 5, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 59.70%. Comparing base (6ddc2b4) to head (832efaf).
⚠️ Report is 929 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #5299       +/-   ##
===========================================
+ Coverage   47.24%   59.70%   +12.45%     
===========================================
  Files         476      589      +113     
  Lines       46892    63490    +16598     
===========================================
+ Hits        22154    37906    +15752     
- Misses      24738    25584      +846     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

unreachable!("there should be a string at index")
};
self.ic.push(InlineCache::new(name.clone()));
let is_length = *name == StaticJsStrings::LENGTH;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

hmm do you really need to introduce this? doesnt look necessary if you dont override name below.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-VM Issues and PRs related to the Boa Virtual Machine. Waiting On Review Waiting on reviews from the maintainers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants