Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prune collected shared shapes #2941

Merged
merged 2 commits into from
May 29, 2023
Merged

Prune collected shared shapes #2941

merged 2 commits into from
May 29, 2023

Conversation

HalidOdat
Copy link
Member

@HalidOdat HalidOdat commented May 18, 2023

This PR prunes all transitions in a SharedShape when a transition's WeakGc has been garbage collected and when we insert 255 properties/prototypes.

Example with debug print on prune:

>> ({a: 1})
{
   a: 1
}
>> ({a: 1, b: 2})
{
   a: 1,
   b: 2
}
>> $boa.gc.collect() // Trigger a garbage collection.
undefined
>> ({a: 1})
DEBUG PRINT - Pruning property transitions
{
   a: 1
}
>>

@HalidOdat HalidOdat added the execution Issues or PRs related to code execution label May 18, 2023
@HalidOdat HalidOdat added this to the v0.17.0 milestone May 18, 2023
@HalidOdat HalidOdat requested a review from a team May 18, 2023 04:19
@github-actions
Copy link

github-actions bot commented May 18, 2023

Test262 conformance changes

Test result main count PR count difference
Total 93,982 93,982 0
Passed 73,783 73,783 0
Ignored 17,489 17,489 0
Failed 2,710 2,710 0
Panics 0 0 0
Conformance 78.51% 78.51% 0.00%

@codecov
Copy link

codecov bot commented May 18, 2023

Codecov Report

Merging #2941 (e2b203b) into main (4a368a2) will increase coverage by 0.03%.
The diff coverage is 80.85%.

@@            Coverage Diff             @@
##             main    #2941      +/-   ##
==========================================
+ Coverage   50.39%   50.42%   +0.03%     
==========================================
  Files         443      444       +1     
  Lines       45337    45392      +55     
==========================================
+ Hits        22846    22889      +43     
- Misses      22491    22503      +12     
Impacted Files Coverage Δ
boa_engine/src/object/shape/shared_shape/mod.rs 84.15% <33.33%> (-0.85%) ⬇️
...rc/object/shape/shared_shape/forward_transition.rs 86.53% <81.57%> (-13.47%) ⬇️
boa_gc/src/pointers/ephemeron.rs 92.98% <100.00%> (+0.52%) ⬆️
boa_gc/src/pointers/weak.rs 70.58% <100.00%> (+3.92%) ⬆️

... and 10 files with indirect coverage changes

Copy link
Member

@raskad raskad left a comment

Choose a reason for hiding this comment

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

Do you think it makes sense to have a test for this behaviour?

@HalidOdat
Copy link
Member Author

Do you think it makes sense to have a test for this behaviour?

will try to add the tests, and documentation on the two prune trigger methods :)

@HalidOdat HalidOdat force-pushed the prune-collected-shared-shapes branch from 36d61ca to e2b203b Compare May 19, 2023 04:56
@HalidOdat HalidOdat requested review from raskad and a team May 20, 2023 17:00
Copy link
Member

@raskad raskad left a comment

Choose a reason for hiding this comment

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

Really nice improvement!

@raskad raskad requested a review from a team May 27, 2023 14:58
Copy link
Member

@jedel1043 jedel1043 left a comment

Choose a reason for hiding this comment

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

LGTM!

@jedel1043 jedel1043 added this pull request to the merge queue May 29, 2023
Merged via the queue into main with commit c013cac May 29, 2023
@HalidOdat HalidOdat deleted the prune-collected-shared-shapes branch May 29, 2023 17:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
execution Issues or PRs related to code execution
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants