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

release-19.1: roachpb: refine Transaction proto cloning #36114

Merged

Conversation

nvanbenschoten
Copy link
Member

Backport 2/2 commits from #35762.

/cc @cockroachdb/release


Fixes #35803.

This PR includes the final two commits from #35719.

By making Transaction.ObservedTimestamps immutable (which it almost already was), we can prohibit all interior mutability of references within Transaction, give it value semantics, and eliminate the distinction between "shallow" and "deep" object cloning. This reduces the cost of a clone to a single straightforward allocation and makes working with the object easier to think about.

cc. @tbg

It turns out that it was easy enough to disallow interior mutability
entirely within a Transaction and treat all of its references as
immutable. Doing so allows us to only ever need to perform shallow
clones of Transaction objects reducing the cost of a clone to a single,
fixed-size allocation.

Release note: None
Now that Transaction.Clone doesn't perform any deep copying, it can
serve the role it should have had of simply cloning a Transaction to
a new object on the heap. Callers who only need a value can use
Transaction's full value semantics directly and simply copy it by value.

Release note: None
@nvanbenschoten nvanbenschoten requested review from tbg and a team March 25, 2019 19:44
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@tbg
Copy link
Member

tbg commented Mar 25, 2019 via email

@nvanbenschoten nvanbenschoten merged commit 7f8a096 into cockroachdb:release-19.1 Mar 25, 2019
@nvanbenschoten nvanbenschoten deleted the backport19.1-35762 branch March 25, 2019 23:08
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.

3 participants