Skip to content

Commit

Permalink
Perform lambda patch in its own parallel walk
Browse files Browse the repository at this point in the history
Summary:
## Context

The 1st parallel [walk](https://www.internalfb.com/code/fbsource/[a5da2b900a795b6004c92bc57575d3d697445fda]/fbandroid/native/redex/opt/typedef-anno-checker/TypedefAnnoPatcher.cpp?lines=434) in the patcher tries to do too many things in one go. It could patch the same method/field in different threads via different code paths. the same field/method if patched via different code path can trigger separate logic after the patching event causing things to diverge.

## Proposal

It is still not clear to me what is the exact piece of logic that introduced the nondeterminism. But it's obvious to me that breaking things up into steps should help.

In this change, I move the more localized and straightforward patching logic into its own parallel walk. As a result, we move the lambda patching logic into its own walk minimizing the adverse interactions between the two steps.

Reviewed By: wsanville

Differential Revision: D70307382

fbshipit-source-id: d249e0da03eba64fb607b3fc400c4be9ecbb7ed0
  • Loading branch information
Wei Zhang (Devinfra) authored and facebook-github-bot committed Feb 27, 2025
1 parent cbcc9df commit 623bb15
Showing 1 changed file with 18 additions and 11 deletions.
29 changes: 18 additions & 11 deletions opt/typedef-anno-checker/TypedefAnnoPatcher.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -436,6 +436,24 @@ void TypedefAnnoPatcher::run(const Scope& scope) {
if (is_enum(cls) && type::is_kotlin_class(cls)) {
fix_kt_enum_ctor_param(cls, class_stats.fix_kt_enum_ctor_param);
}
for (auto m : cls->get_all_methods()) {
patch_parameters_and_returns(
m, class_stats.patch_parameters_and_returns);
patch_synth_methods_overriding_annotated_methods(
m, class_stats.patch_synth_methods_overriding_annotated_methods);
if (is_constructor(m) &&
has_typedef_annos(m->get_param_anno(), m_typedef_annos)) {
patch_synth_cls_fields_from_ctor_param(
m, class_stats.patch_synth_cls_fields_from_ctor_param);
}
}
return class_stats;
});

m_patcher_stats +=
walk::parallel::classes<PatcherStats>(scope, [this](DexClass* cls) {
auto class_stats = PatcherStats();

if (is_synthesized_lambda_class(cls) || is_fun_interface_class(cls)) {
std::vector<const DexField*> patched_fields;
for (auto m : cls->get_all_methods()) {
Expand Down Expand Up @@ -480,17 +498,6 @@ void TypedefAnnoPatcher::run(const Scope& scope) {
});
}
}
for (auto m : cls->get_all_methods()) {
patch_parameters_and_returns(
m, class_stats.patch_parameters_and_returns);
patch_synth_methods_overriding_annotated_methods(
m, class_stats.patch_synth_methods_overriding_annotated_methods);
if (is_constructor(m) &&
has_typedef_annos(m->get_param_anno(), m_typedef_annos)) {
patch_synth_cls_fields_from_ctor_param(
m, class_stats.patch_synth_cls_fields_from_ctor_param);
}
}
return class_stats;
});

Expand Down

0 comments on commit 623bb15

Please sign in to comment.