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

[pull] main from facebook:main #533

Open
wants to merge 523 commits into
base: main
Choose a base branch
from
Open

[pull] main from facebook:main #533

wants to merge 523 commits into from

Conversation

pull[bot]
Copy link

@pull pull bot commented Apr 17, 2024

See Commits and Changes for more details.


Created by pull[bot]

Can you help keep this open source service alive? 💖 Please sponsor : )

@pull pull bot added the ⤵️ pull label Apr 17, 2024
itang00 and others added 29 commits September 4, 2024 14:46
Summary: Revert this due to T200614595

Reviewed By: thezhangwei

Differential Revision: D62189033

fbshipit-source-id: 09513cec3b8ece1c0617eebaaa07fbcda5fbbd7c
Summary: Revert D61681031 to address T200614595

Reviewed By: thezhangwei

Differential Revision: D62197730

fbshipit-source-id: 855f0eecc788377da08c307ce8bc554745d501ec
Summary: Follow https://cs.android.com/android/platform/superproject/main/+/main:art/libdexfile/dex/dex_instruction_list.h

Reviewed By: agampe

Differential Revision: D62214420

fbshipit-source-id: 4382260a9bbb308731f37185441f03077fccb6d4
Reviewed By: zertosh

Differential Revision: D62244631

fbshipit-source-id: e13f6cb400144f16f68400806b736edf7615ab4d
Summary: There are is_throw and can_throw method

Reviewed By: NTillmann

Differential Revision: D62260532

fbshipit-source-id: 28649dfa0cdaff8169585aacf3e19c4a29aeb30a
Summary: Log how many methods we expect to get compiled, and how many code units are in them.

Differential Revision: D61233194

fbshipit-source-id: 1d0c26d6d252b64703552796eac0338c5d606091
Summary: These options are not used. This is a behavior-preserving change.

Differential Revision: D61364189

fbshipit-source-id: d83c1251b676497b990dab8ed5b0711f48697d59
Summary:
This diff addresses a TODO in the code:
```
// TODO: This code shouldn't live here. "baseline_profile_config" should likely
//       be part of the GlobalConfig and parsed early on (close to where deep
//       data profiles and the betamap get read). Then both InterDexPass and
//       ArtProfileWriterPass (and anything else) can read the baseline profile
//       config for a build easily.
//       Doing it here for now for ease/speed of implementation for an upcoming
//       APK test.
```

This is a behavior-preserving change.

Reviewed By: jimmycFB

Differential Revision: D61364190

fbshipit-source-id: ec86623a237be272f78339aea715ece154f46143
Summary:
We log how many method-refs have no defs.

Filtering that out has no actual effect, as these methods do not exist in the final APK.

Reviewed By: thezhangwei

Differential Revision: D61367977

fbshipit-source-id: 346a26575a9cf5b3a345e2b9fd946cbb180364d7
Summary:
This pulls out the concept of a baseline profile (in order to replace the legacy algo in Redex with different one in a later diff).

This is a behavior-preserving change.

Differential Revision: D61367976

fbshipit-source-id: 8a79b9c9e41eb537964f6e825cb4dd5ffd32351d
Summary: This new function isn't used yet, so this is a behavior-preserving diff.

Reviewed By: thezhangwei

Differential Revision: D61370274

fbshipit-source-id: 0fa93870042412a800e31a04a8597884db7301bf
Summary:
The legacy mode is on by default, so this is a behavior-preserving change.

What legacy mode is set to off, the global baseline profile config is used.

Reviewed By: ssj933

Differential Revision: D61370276

fbshipit-source-id: e6dc6f1b78e23417386c3893694a83c05edecdd7
Summary: This is a behavior-preserving change as the option is on by default.

Reviewed By: thezhangwei

Differential Revision: D61309997

fbshipit-source-id: 360af3d9077a520965a8e03b674be2a00ca896f8
Summary: This is basically the same as D61681033. The only difference is `add_param_annotations`, a helper function that I use in four places. `add_param_annotations` checks that we're not adding duplicate annotations to parameters.

Reviewed By: thezhangwei

Differential Revision: D62215312

fbshipit-source-id: ee1389f91725ed16fa13f95ba74b79c900032bf2
Summary: Basically the same as D61681031. The only difference is use of the helper function `add_param_annotations` that I added in the previous diff to prevent duplicate parameter annotations.

Reviewed By: thezhangwei

Differential Revision: D62215313

fbshipit-source-id: 407c4b9e1ffa40bfdcf868ddf5e8ba62f5cdf575
Summary: This helps identify changes in the order in the next diffs.

Reviewed By: ssj933

Differential Revision: D61421527

fbshipit-source-id: 474572e1dd09de33242e102fce2ee51004a37b93
Summary:
Whatever is configured is ordered as such, followed by all other interactions which will get ordered by their string ids.
The default doesn't change in this diff, so this is a behavior-preserving change.

Reviewed By: agampe

Differential Revision: D61421525

fbshipit-source-id: 1268acff4921fc0c96baa96ddeee0d283a480639
Summary:
New metrics!

When partition_by_hotness is True, then this also orders the cases by interaction order. But it's False by default, so this is a behavior-preserving diff.

Reviewed By: ssj933

Differential Revision: D61309183

fbshipit-source-id: 9ec54ba9eea7c5800f83cf6eb5594dc9a1ba3933
Summary:
This is a big overhaul of the MethodSplittingPass configuration:
- It introduces new thresholds for “hot” methods (which we really want to split for performance reasons).
- It reconfigures the defaults in a way I have found more reasonable in order to disable the SplitHugeSwitchPass later.

The actual method-splitting functionality is not affected by this threshold tuning.

Reviewed By: ssj933

Differential Revision: D61310000

fbshipit-source-id: a74e377ec3983118d2f919a30fc1707159b29ba7
Summary:
The method splitting pass has the ability to destroy formerly packed switches (which is undesirable). This adds logging on how often that happens.

Besides adding new metrics, this is a behavior-preserving change.

Differential Revision: D62275226

fbshipit-source-id: 764363b73d4348ae759eb9ab16b8095589ecfa47
Summary: For packed switches, we first check if there is a way to perform the desired splitting by chopping off from the back of a packed switch, then from the front, and only if neither works, then we use the aggregation algorithm that aims at finding cases with most code overlap.

Differential Revision: D62279630

fbshipit-source-id: 244e41b7e030ffddfdbe427591da34f860cbf038
Summary: The aggregation optimization that finds switch cases with maximal overlap gets biased by de-duplicated trailing return statements. We effectively undo that deduping here to get a more reasonable aggregation.

Differential Revision: D62262452

fbshipit-source-id: 8ff6d055a4003be6d3af0ebeb1ad1eeb0f4b347c
Summary: Patch Java setters, which have a pattern like `access$002`

Reviewed By: thezhangwei

Differential Revision: D61729203

fbshipit-source-id: fe72806f23aa10bf817fec56fd9061790d086018
Summary:
The bug with `boost::container::flat_map::extract_sequence` has been fixed (see boostorg/container#288)
This should go out in boost 1.87 (probably in a few months).

Therefore, let's re-enable the optimization for boost versions != 1.86

Reviewed By: arnaudvenet

Differential Revision: D62436361

fbshipit-source-id: b2010fbae81f4b65039ee1dd6459c5e7021dbc3f
Summary: workaround for the ModelGen getters and setters in the checker for now

Reviewed By: thezhangwei

Differential Revision: D62610770

fbshipit-source-id: 446b3136a2c7b4b8a62c320c50f8af17a4efd657
Summary: We used to consider spurious "uses" of a value flowing into a split-out code fragment when determining the type of synthesized parameter of the synthesized split method. This is getting fixed here.

Reviewed By: thevinster

Differential Revision: D62662913

fbshipit-source-id: 3502c121492b26b3f06f5857fd627e2cfb25c045
Summary: Within a basic block, a cold source block followed by a hot source block should be a violation. This updates our violation metric to more accurately track this.

Reviewed By: jimmycFB

Differential Revision: D62194678

fbshipit-source-id: d4683428c63a79dd588b033b93dafdb10ec46e03
Summary: Take a look at the test case that explains what this is trying to achieve. Without the change to Inliner.cpp, `LX;.<init>:()V` will get inlined and that is bad given how the stores are set up.

Reviewed By: ssj933

Differential Revision: D62671372

fbshipit-source-id: 64737cba4e7aabd1f7cbe4b5f8a1aaed18565298
Summary: If a synthetic bridge method calls an annotated method, annotated the synthetic method

Reviewed By: thezhangwei

Differential Revision: D63067523

fbshipit-source-id: e461075b37ad84f42ae2c717476b0265153c54cc
agampe and others added 30 commits December 2, 2024 12:12
Summary: As title.

Reviewed By: NTillmann

Differential Revision: D66661531

fbshipit-source-id: a8c09a6ea918a456b594a1dce3318042ca3a8871
Summary: Instead of using `at`, explicitly look for the element so an INVALID_DEX assert can be triggered.

Reviewed By: NTillmann

Differential Revision: D66662340

fbshipit-source-id: 582ca94034e43433008b8020305ca12554babac6
Summary:
This is a mechanical refactoring.
This is a behavior-preserving change.

Reviewed By: ssj933

Differential Revision: D66525537

fbshipit-source-id: 91e60199cbd0ca0ed77972a3810a1eb39f5c9602
Summary:
The assertion that is removed here was put in a place earlier (D46806422) as part of a refactoring to implement proper memory management.
I don't see a deep reason for disallowing copying of instruction with data. So here, we implement this functionality by cloning the data.

Reviewed By: ssj933

Differential Revision: D66545211

fbshipit-source-id: 617fb708520eb683fe0cc3a482dbb741e1d00613
Reviewed By: ssj933

Differential Revision: D66525536

fbshipit-source-id: 3077eb8eaf0805344b1898d4afec89adb3dfb633
Summary:
Refactor the constructor to take constructed objects so as not to leak on failure.

Check dex types involved in the construction:
* Should not be null
* Must be reference types
* Must not be array types

Reviewed By: NTillmann

Differential Revision: D66664148

fbshipit-source-id: 665072c8726d9574bf434b95b3539c0caaf65b5a
Summary: Remove `default` case. Fill in the missing cases.

Reviewed By: NTillmann

Differential Revision: D66667415

fbshipit-source-id: a2c03c1049704dbc225a7c5073f51ed6f9747e67
Summary: As title. Denote invalid class or jar inputs.

Reviewed By: NTillmann

Differential Revision: D66671956

fbshipit-source-id: 6f4b59c8197219ccfda0cf47639c44df44bc6609
Summary: As title.

Reviewed By: NTillmann

Differential Revision: D66672107

fbshipit-source-id: 12b70340c1f215a6ecc00ef52bbfd464ea65d48e
Summary: As title.

Reviewed By: NTillmann

Differential Revision: D66674640

fbshipit-source-id: 6fca0e8519a633a5fe13acfada33e3efdee97eb9
Summary: When it comes to enforcing assignments from synthetic fields, the checker already does a good job for fields on lambdas. However, it doesn't seem like we fully support synthetic fields on fun interface classes, like the one shown in this [diff](https://www.internalfb.com/diff/D59566990?dst_version_fbid=1151809069839783). Before landing a precise fix for this particular issue. I propose to relax the safety check sourcing from a synthetic field. In general, this is fairly safe.

Reviewed By: wsanville

Differential Revision: D66682022

fbshipit-source-id: ac53defeb0c8d269a48c4bc2b0c089eb38f4a059
Summary: Refactor existing checks into helper function and use consistently.

Reviewed By: ssj933

Differential Revision: D66739139

fbshipit-source-id: aa22a81813c61e21482df1c18519bc9f3ef49754
Summary:
`IRInstruction::hash` already expanded the `data` portion of an `IRInstruction`. This makes it so that `operator==` also does a deep equality check over the `data`.

This addresses an internal inconsistency in the branch-prefix hoisting pass.

Reviewed By: wsanville

Differential Revision: D66800414

fbshipit-source-id: 68667e96e2e08b9328f2ef29b5dff45818465c9d
Summary: Group all the files of the test into one place.

Reviewed By: NTillmann

Differential Revision: D66731110

fbshipit-source-id: ab5cdb382f255a2b115ab1d0a8b3d027eca66dd9
Summary:
Any `dex_insn` in a `MethodItemEntry` is now owned and deleted when the entry goes away.

Fix up `sync`-ing to ensure single owner.

Reviewed By: NTillmann

Differential Revision: D66560537

fbshipit-source-id: 3d66a0421343f3568e204b14f60ffbe38899a0dd
… in the call back method

Summary:
The checker has the logic to patch an anonymous class if the call back method requires a typedef value sourcing from its fields. The patching involves patching the synthetic fields and the matching ctor params. However, the logic is currently gated by adhoc method name matching. It doesn't cover everything it should cover.

In this change, we replace the method name matching logic with more rigorous class structure pattern matching specifically for kotlinc style lambda and fun interface classes. They are the intended targets for the above mentioned class patching logic.

Reviewed By: itang00

Differential Revision: D66730722

fbshipit-source-id: a2a87a6db35bc0299b65734d8e4187254f28d2be
Summary: As title. Add tests.

Reviewed By: NTillmann

Differential Revision: D66770190

fbshipit-source-id: 463d169e6dea66d92f1c13575697281edfc00a03
Summary: As title. Add tests.

Reviewed By: NTillmann

Differential Revision: D66770189

fbshipit-source-id: f969dcd7d8230b85d050fc52e1a3dcd4470d76f5
Summary: Object array stores are type-checked in a very relaced fashion and checked at runtime.

Reviewed By: NTillmann

Differential Revision: D66770707

fbshipit-source-id: a12fddc7873455e62d19e1c56dbc698418d2b1ec
Summary:
In order to rewire the baseline profile infra to preprocessor -> redex -> postprocessor, we need to supply the output of the preprocessor as an input into redex.

This diff changes the buck files and also adds the output directory of the preprocessor as an input into redex.

Currently this is unused in redex, but the next diff will be constructing the baseline profile off of the preprocessed base.

Reviewed By: jimmycFB

Differential Revision: D65979598

fbshipit-source-id: b47037895e0864275a6dacedd896cf812fbe9b09
Summary:
Fun interface class can have additional synthetic bridge method aside from the real call back one. P1692891678 shows an example.
We also need to patch them just like the single method ones. The check we introduced in D66730722 is a bit too restictive.

Reviewed By: itang00

Differential Revision: D66903309

fbshipit-source-id: 68eb0568b4fed8c1f45faaa5bfab20178026c9ee
Summary: This removes all references to locator strings.

Differential Revision: D66683895

fbshipit-source-id: e5d0d36a656598dddeae086f2441261142ea33d7
Summary: Ensure the instrumentation-signalling field has `0` value, and remove any now-invalid field-writes.

Reviewed By: NTillmann

Differential Revision: D66846034

fbshipit-source-id: 9053cbbb0a4fa7465b713c7b617cb03a2bba69a9
Summary: Unset more keeps when not instrumenting.

Reviewed By: NTillmann

Differential Revision: D66846037

fbshipit-source-id: 086abd056de9b48735c036b44fcac1b8534d7a61
Summary: As title.

Reviewed By: itang00

Differential Revision: D66967385

fbshipit-source-id: 318ce81048f9b68da109374056bcad2f31d7a27a
Summary: Add an optional check that all types with a given prefix are defined.

Reviewed By: danjin250

Differential Revision: D66666167

fbshipit-source-id: e99709feee2a6b34ed63bf50a4236afa987a2b94
Summary:
Updates the redex baseline profile config to the new 2.0 format. After talking to Nikolai, it looks like we probably do need this config instead of the metadata file.

We will probably have to update whatsapp before making this change

Reviewed By: jimmycFB

Differential Revision: D66207921

fbshipit-source-id: e59cb0a0177733c43f600619266fac7a1d3e7449
Summary: Modify redex baseline config parser to take in a list of baseline configs instead of a single config.

Reviewed By: jimmycFB

Differential Revision: D66314528

fbshipit-source-id: 67e0b085768adf6ec9f26ea5a325153d57bd0abe
Summary:
Add a pass to optimize `valueOf`.

The files TestIntdef.java and TestStringDef.java represent the Falco enums that will have a default `valueOf` method for debug builds and a `valueOfOpt` method that will be edited by this pass. The `map_encoding` string will be filled with the StringTreeMap encoding.

After editing `valueOfOpt`, set it to `public` and switch out all invokes of `valueOf` with `valueOfOpt`

Reviewed By: thezhangwei

Differential Revision: D65912448

fbshipit-source-id: c36177218356ea6cddc1e5df261e7951407aab2a
Summary: When auto scanning the scope and identifying suitable class hierarchies for merging, we skip the ones where the root interface definition is missing.

Reviewed By: wsanville

Differential Revision: D67216886

fbshipit-source-id: 3b9a840232d6adeda6eb9f561030a248dfa6cd38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants