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

Support trimming distributed transactions if unused, without a feature switch. #3

Merged
merged 1 commit into from
Sep 30, 2022

Conversation

teo-tsirpanis
Copy link

This PR allows trimming away the distributed transactions code if the newly introduced property TransactionManager.ImplicitDistributedTransactions is not set. An ILLink feature switch is not needed. I also added [RequiresUnreferencedCode] to the relevant call chain, up to the aforementioned property's setter, and removed a suppression of this warning in DtcProxyShimFactory.ConnectToProxyCore.

Copy link
Owner

@roji roji left a comment

Choose a reason for hiding this comment

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

@teo-tsirpanis thanks, this is really nice!

Do I understand correctly that the linker can see that s_transactionConnector will never be non-null unless ImplicitDistributedTransactions.set is called? I wasn't aware the linker could do this, is this some special allowance for static variables (since otherwise just looking at ConnectToProxy there's no context for knowing when s_transactionConnector is null etc.).

@roji roji merged this pull request into roji:DistributedTransactionsOptin Sep 30, 2022
@teo-tsirpanis teo-tsirpanis deleted the trimmable-dtc branch September 30, 2022 19:36
@teo-tsirpanis
Copy link
Author

teo-tsirpanis commented Sep 30, 2022

The linker sees that the ITransactionConnector.ConnectToProxyCore will be called, but that's just an interface method and won't keep any code by itself. If we create a concrete class -which happens in set_ImplicitDistributedTransactions- the linker will connect the dots and keep the concrete class' method's ConnectCore.

This also means that if we just set ImplicitDistributedTransactions to true without creating any transaction doing whatever calls DtcProxyShimFactory.ConnectToProxy (😅), the distributed transactions code will be still trimmed. It's like an AND gate for the linker: to keep the code we need to both call the interface's method (which happens in DtcProxyShimFactory.ConnectToProxy), and create an object that implements it (which happens in set_ImplicitDistributedTransactions). The advantage about it is that only one of the "gate's" members needs to be marked as trimmer-unsafe.

@roji
Copy link
Owner

roji commented Sep 30, 2022

Thanks for the explanation!

But re the 2nd point, I don't think the linker can actually see that ConnectToProxy is only called when a distributed transaction is started... The fundamental API design of System.Transactions is that there's no explicit gesture for starting a distributed transaction: that's something that happens implicitly when you e.g. enlist two database connections to the same Transaction (e.g. via a TransactionScope). So I think the code wouldn't be trimmed the moment the flag is set (which is totally fine, of course).

@roji
Copy link
Owner

roji commented Sep 30, 2022

(In other words, if the linker could know to trim the code because no distributed transaction is started in an app, we wouldn't need the flag any more :))

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.

2 participants