Bug 49087 - [mtouch] Computing list of assemblies is slow
Summary: [mtouch] Computing list of assemblies is slow
Alias: None
Product: iOS
Classification: Xamarin
Component: Tools ()
Version: master
Hardware: PC Mac OS
: High enhancement
Target Milestone: 15.3
Assignee: Rolf Bjarne Kvinge [MSFT]
Depends on:
Reported: 2016-12-06 13:30 UTC by Rolf Bjarne Kvinge [MSFT]
Modified: 2017-07-14 11:29 UTC (History)
4 users (show)

Is this bug a regression?: ---
Last known good build:

Notice (2018-05-24): bugzilla.xamarin.com is now in read-only mode.

Please join us on Visual Studio Developer Community and in the Xamarin and Mono organizations on GitHub to continue tracking issues. Bugzilla will remain available for reference in read-only mode. We will continue to work on open Bugzilla bugs, copy them to the new locations as needed for follow-up, and add the new items under Related Links.

Our sincere thanks to everyone who has contributed on this bug tracker over the years. Thanks also for your understanding as we make these adjustments and improvements for the future.

Please create a new report on Developer Community or GitHub with your current version information, steps to reproduce, and relevant error messages or log files if you are hitting an issue that looks similar to this resolved bug and you do not yet see a matching new report.

Related Links:

Description Rolf Bjarne Kvinge [MSFT] 2016-12-06 13:30:37 UTC
Computing the list of assemblies is slow, because we load everything in memory to check for custom attributes that has type references to other assemblies: https://github.com/xamarin/xamarin-macios/blob/7dac4dfe5806467fb47f085089498fcf18e2cbf5/tools/mtouch/Target.cs#L279-L288

For fully cached rebuilds this takes a significant amount of time (1.4s of 2s (70%)), and we do it twice for fat (32+64 bits) apps.

Ideas to improve this:

* Implement an API in Cecil that iterates directly on the table of custom attributes, without having to load every parent element.
* Cache the dependencies for an assembly.
* Don't check custom attributes for assemblies we ship (and of course also verify that we don't need to scan those attributes, and add a test that triggers if this ever changes in the future).
Comment 2 Rolf Bjarne Kvinge [MSFT] 2016-12-08 12:34:33 UTC
Cecil added API which makes this faster: https://github.com/jbevain/cecil/commit/8e0f750e885ca8b3262996d16a9bad80d78f309c

Using this new API like this: https://gist.github.com/a4103ebac349f899803d6d84fb5b5593, makes my test case 2x faster (from 2s to 1s).

However this API can only be used once mono/master bumps cecil, and we start using mono/master again, so we'll have to wait a bit.
Comment 5 Rolf Bjarne Kvinge [MSFT] 2017-01-19 07:27:59 UTC
master: https://github.com/xamarin/xamarin-macios/pull/1531
Comment 6 Rolf Bjarne Kvinge [MSFT] 2017-01-19 09:40:11 UTC
The PR to use Cecil's new API has been merged in master: https://github.com/xamarin/xamarin-macios/commit/3b310b71ebc35d21706d6e45655c5c92c47da649

I'm leaving this bug open, because we're still using significant time (about 50% of the total time when invoking mtouch when mtouch doesn't have to do anything) iterating assembly attributes, so we might get additional improvements by implementing some of the other suggestions (cache results, skip some assemblies).
Comment 7 Sebastien Pouliot 2017-01-20 14:32:29 UTC
> * Don't check custom attributes for assemblies we ship

I like that, it would be easy to pre-compute/codify the dependencies for all the SDK assemblies we ship. That logic code could be shared with `mtouch` unit tests to ensure it remains as valid.

We already have similar tests to detect "new" facades and more recently for the inliner.
Comment 8 Rolf Bjarne Kvinge [MSFT] 2017-01-20 14:35:57 UTC
(In reply to Sebastien Pouliot from comment #7)
> > * Don't check custom attributes for assemblies we ship
> I like that, it would be easy to pre-compute/codify the dependencies for all
> the SDK assemblies we ship. That logic code could be shared with `mtouch`
> unit tests to ensure it remains as valid.

In fact we can just automatically precompute the dependencies in our build, and ship that information. Then there would be nothing to update manually.

I like this idea, we can just generate C# code with the information we need, and compile it directly into mtouch/mmp :D
Comment 9 Rolf Bjarne Kvinge [MSFT] 2017-05-23 13:34:02 UTC Comment hidden (obsolete)
Comment 10 Rolf Bjarne Kvinge [MSFT] 2017-05-23 13:34:33 UTC
Cache the list of assemblies: https://github.com/xamarin/xamarin-macios/pull/2120
Don't look for assembly references in attributes in assemblies we ship: https://github.com/xamarin/xamarin-macios/pull/2121
Comment 12 Prasad Raghorte 2017-07-14 11:28:20 UTC
Hi Rolf, I seems like an unit testing bug to me, if not please provide steps to verify this defect.
Comment 13 Rolf Bjarne Kvinge [MSFT] 2017-07-14 11:29:16 UTC
This is an internal performance improvement only, it has no testable customer-visible change, so I'm marking this as VERIFIED.