This is Xamarin's bug tracking system. For product support, please use the support links listed in your Xamarin Account.
Bug 49087 - [mtouch] Computing list of assemblies is slow
Summary: [mtouch] Computing list of assemblies is slow
Status: CONFIRMED
Alias: None
Product: iOS
Classification: Xamarin
Component: Tools (show other bugs)
Version: master
Hardware: PC Mac OS
: High enhancement
Target Milestone: 15.3
Assignee: Rolf Bjarne Kvinge
URL:
Depends on:
Blocks:
 
Reported: 2016-12-06 13:30 UTC by Rolf Bjarne Kvinge
Modified: 2017-03-23 14:45 UTC (History)
3 users (show)

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


Attachments

Description Rolf Bjarne Kvinge 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 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 2017-01-19 07:27:59 UTC
master: https://github.com/xamarin/xamarin-macios/pull/1531
Comment 6 Rolf Bjarne Kvinge 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 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

Note You need to log in before you can comment on or make changes to this bug.