Replace NanBoxing's Box with a refcount and improve clone/drop performance

[hansl]

Essentially what I'm doing here is instead of using a Box as the container for the JsValue's inner NaN-boxed pointer, I use an RC. When cloning the value, that RC will the increment (and decrement on drop) instead of allocating a new Box on the heap. This should make clones of JsValue simply increment an integer in memory.

So given the following:

/* 1 */ let a = JsObject::new();
/* 2 */ let b = JsValue::from(a.clone());
/* 3 */ let c = b.clone();
/* 4 */ drop(b);
/* 5 */ drop(a);
/* 6 */ drop(c);

The refcount of the JsObject itself will be: 1 at line 1, 2 at line 2, still 2 at line 3 as the clone of the JsValue itself increment the internal counter, 2 at line 4, 1 at line 5 (a will drop, but the JsValue itself will still have an inner JsObject), then finally 0 and dropped at line 6.

[Copilot]

Pull Request Overview

This PR updates the memory management for nan boxed values by replacing Box with Arc.

• The tagging functions now accept Arc instead of Box for JsBigInt, JsObject, JsSymbol, and JsString.
• The clone and drop implementations are updated to manually increment and decrement the Arc reference count.

Comments suppressed due to low confidence (2)

core/engine/src/value/inner/nan_boxed.rs:461

• [nitpick] Consider refactoring the clone implementation to use Arc::clone() directly instead of manually calling Arc::increment_strong_count(), as this would simplify the code and reduce the risk of mismatches in the reference count.

if self.is_object() {

core/engine/src/value/inner/nan_boxed.rs:730

• [nitpick] Using Arc::decrement_strong_count() manually in the drop implementation is error-prone; consider using Arc::from_raw() to properly handle deallocation once the reference count drops to zero, ensuring correct memory management.

if self.is_object() {

[codecov]

Codecov Report

Attention: Patch coverage is 86.20690% with 4 lines in your changes missing coverage. Please review.

Project coverage is 52.67%. Comparing base (6ddc2b4) to head (784fb83).
Report is 397 commits behind head on main.

Files with missing lines	Patch %	Lines
core/engine/src/value/inner/nan_boxed.rs	86.20%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4219      +/-   ##
==========================================
+ Coverage   47.24%   52.67%   +5.43%     
==========================================
  Files         476      487      +11     
  Lines       46892    51966    +5074     
==========================================
+ Hits        22154    27375    +5221     
+ Misses      24738    24591     -147     

☔ 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.

[Copilot]

Pull Request Overview

This PR aims to improve the pointer handling in NanBoxedValue by replacing the Box-based management with reference counting. Key changes include:

• Switching from Box to Rc for various value types (JsBigInt, JsObject, JsSymbol, and JsString)
• Modifying clone and drop implementations to use unsafe Rc reference count manipulation

[HalidOdat]

Suggestion: Maybe we should pass JsBigInt as value directly since it's wrapped in an Rc<RawBigInt> already internally, and add methods for into_raw()/from_raw() to JsBigInt.

[hansl]

This cannot work unless I change the interface of the JsValue to return Rc<...> (or similar) instead of references. To return a &'a JsBigInt with your suggested change I'd need to reconstruct it on the stack, and I cannot return a reference to that. Unless I'm missing something?

[HalidOdat]

I see, sorry I missed that 😅 , we could get around this by using a bit of safe "unsafe" code by adding #[repr(transparent)] on JsBigInt and transmute-ing &'a Rc<RawBigInt> into &'a JsBigInt JsBigInt.

For more info see: https://doc.rust-lang.org/nomicon/other-reprs.html#reprtransparent

[hansl]

I kind of tried that. I need to spend more time on it, but I was still getting segfaults accessing freed or invalid memory.