perf: remove GcRefCell from inline cache to avoid borrow checking overhead

[mansiverma897993]

This Pull Request fixes/closes #5399.

It changes the following:

• Replaces GcRefCell<ArrayVec<CacheEntry, PIC_CAPACITY>> with Cell<ArrayVec<CacheEntry, PIC_CAPACITY>> in InlineCache structure.
• Uses Cell::take() and Cell::set() to access and modify the inline cache entries in a zero-overhead manner, avoiding all dynamic borrow-checking overhead on the hot path.
• Manually implements Clone and Debug for InlineCache because Cell<T> only auto-derives them when T: Copy, and ArrayVec is non-Copy.
• Replaces .entries.borrow() with a new test helper .entries() in core/engine/src/vm/inline_cache/tests.rs to allow unit testing without borrow checks.

[github-actions]

Test262 conformance changes

Test result	main count	PR count	difference
Total	53,125	53,125	0
Passed	51,072	51,072	0
Ignored	1,482	1,482	0
Failed	571	571	0
Panics	0	0	0
Conformance	96.14%	96.14%	0.00%

Tested main commit: de2221a09c132951c2ebad36e62ecd20b9987215
Tested PR commit: bbe3e96c9646bb2b7515eca2f33794c802df962c
Compare commits: de2221a...bbe3e96

[jasonwilliams]

Thanks for this @mansiverma897993, you need to run rust fmt

[jasonwilliams]

For the clone it feels inefficient to take the value out, put a default one in then add the value back on a hotpath. We may need to get a pointer into the cell here as its safe (we are taking and resetting in the same operation). Something like this:

    fn clone(&self) -> Self {
        // SAFETY: `entries` is only ever accessed through `&self`/`&mut self`
        // on this single-threaded cache, and cloning `CacheEntry` doesn't
        // reenter this `Cell`, so it's safe to read through the raw pointer
        // for the duration of this borrow without disturbing the cell's contents.
        let cloned_entries = unsafe { (*self.entries.as_ptr()).clone() };

        Self {
            name: self.name.clone(),
            entries: Cell::new(cloned_entries),
            megamorphic: self.megamorphic.clone(),
        }
    }

Also, while you're here, can you move the megmorphic to be the first item in the struct? The entries value is so big it pushes the bool out of the cacheline so its a cache miss every time it's read, it may need the #[repr(C)] tag above

[mansiverma897993]

@jasonwilliams I have sucessfully update the PR as per your accordance !! look at once

[codecov]

Codecov Report

❌ Patch coverage is 53.57143% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 60.22%. Comparing base (6ddc2b4) to head (bbe3e96).
⚠️ Report is 982 commits behind head on main.

Files with missing lines	Patch %	Lines
core/engine/src/vm/inline_cache/mod.rs	53.57%	13 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #5400       +/-   ##
===========================================
+ Coverage   47.24%   60.22%   +12.97%     
===========================================
  Files         476      566       +90     
  Lines       46892    63055    +16163     
===========================================
+ Hits        22154    37973    +15819     
- Misses      24738    25082      +344     

☔ View full report in Codecov by Harness.
📢 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.

[jasonwilliams]

@mansiverma897993 thanks, I'm not sure when I can take a look again. In the meantime are you able to compare the benchmarks between main and this? We have a data repo here: https://github.com/boa-dev/data

You can check that out in an adjacent folder and run boa against the bench/bench-v8/combined.js file

When I checked on cachegrind it looked like there were still a high amount of cache misses happening somewhere in that function, maybe @HalidOdat is better at hunting these things down.