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

Delete removed elements from the clone while running GC #103

Merged
merged 1 commit into from
Nov 14, 2020

Conversation

hackerwins
Copy link
Member

@hackerwins hackerwins commented Nov 13, 2020

What type of PR is this?

Uncomment only one /kind <> line, hit enter to put that in a new line, and remove leading whitespace from that line:

/kind api-change

/kind bug

/kind cleanup
/kind deprecation
/kind design
/kind documentation
/kind failing-test
/kind feature

What this PR does / why we need it:

Delete removed elements from the clone while running GC

When initially designing the GC, it was implemented so that only removed
elements from the original were deleted. However, to solve the problem
of accumulating removed elements in the clone, they are also deleted.

Which issue(s) this PR fixes:

Fixes #100 and Related to yorkie-team/yorkie-js-sdk#101

Special notes for your reviewer:

Does this PR introduce a user-facing change?:


Additional documentation:


@dc7303
Copy link
Member

dc7303 commented Nov 14, 2020

As a result of testing, I confirmed that it works normally. 👍

@codecov
Copy link

codecov bot commented Nov 14, 2020

Codecov Report

Merging #103 (3af2077) into master (e737c69) will increase coverage by 0.03%.
The diff coverage is 50.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #103      +/-   ##
==========================================
+ Coverage   55.86%   55.89%   +0.03%     
==========================================
  Files          27       27              
  Lines        2515     2517       +2     
==========================================
+ Hits         1405     1407       +2     
  Misses       1018     1018              
  Partials       92       92              
Impacted Files Coverage Δ
pkg/document/json/rht_pq_map.go 50.70% <0.00%> (ø)
pkg/document/json/root.go 15.55% <ø> (ø)
pkg/document/document.go 43.66% <66.66%> (+1.63%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e737c69...3af2077. Read the comment docs.

When initially designing the GC, it was implemented so that only removed
 elements from the original were deleted. However, to solve the problem
 of accumulating removed elements in the clone, they are also deleted.
Copy link
Member

@dc7303 dc7303 left a comment

Choose a reason for hiding this comment

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

LGTM :)

@hackerwins hackerwins merged commit 6b8e9c5 into master Nov 14, 2020
@hackerwins hackerwins deleted the fix-clone-leak-on-gc branch November 14, 2020 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

It seems that GC of Array and Object is not working properly.
3 participants