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.

Sign up for GitHub

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

Jump to bottom

[sample-tester] Collect app size and build duration statistics. #7602

Merged
merged 41 commits into from
Dec 20, 2019
Merged

[sample-tester] Collect app size and build duration statistics. #7602

merged 41 commits into from
Dec 20, 2019

Conversation

rolfbjarne
Copy link
Member

@rolfbjarne rolfbjarne commented Dec 16, 2019 •
edited
Loading

Continuation of #7517, which got closed when I removed the branch.

This is a work in progress, no need to review yet.

This is ready for review now. Best reviewed by not looking at each commit, but instead at the whole diff at once, since there's a lot of troubleshooting commits.

  • Add a Baseline test which is executed on all the bots. This test is a very basic app we build for device (debug) - we use it as a baseline for how fast the bot is, which means that we don't need to build for more than a single architecture.
  • Adjust the test suite to output performance data for each project built. This is the data collected:
    • Duration of the build
    • Total size of the final .app
    • Size for each file in the final .app
    • Stats about each MSBuild target (duration and frequency)
    • Extra time logging from mtouch
  • Bump ios-samples to get an improved version where it's easier to calculate the final .app directory.
  • Change the yml script to push perf data to xamarin/xamarin-macios-data.
  • Add support for MMP_ENV_OPTIONS to mmp (to mimic MTOUCH_ENV_OPTIONS).

Here is a sample of the data produced: https://github.com/xamarin/xamarin-macios-data/tree/master/perf-data/samples/pull-request/1b847a12bb3bf3c005db18236dd423d461c20e55 (the final last version is slightly different: there won't be a single big file, but several smaller files, but the xml structure is identical).

@rolfbjarne rolfbjarne requested a review from chamons as a code owner December 16, 2019 18:24
@rolfbjarne rolfbjarne added build-package Build (and create package) on internal Jenkins. Apply 'run-internal-tests' to run tests too. do-not-merge Do not merge this pull request not-notes-worthy Ignore for release notes run-sample-tests Run the sample tests skip-all-tests Skip all the tests skip-public-jenkins Completely skip execution in the public Jenkins instance labels Dec 16, 2019
@rolfbjarne

This comment has been minimized.

Sign in to view
@monojenkins

This comment has been minimized.

Sign in to view
chamons
chamons approved these changes Dec 16, 2019
View reviewed changes
@xamarin-release-manager

This comment has been minimized.

Sign in to view
@monojenkins
Copy link
Collaborator

Build success
ℹ️ Skipped execution

@xamarin-release-manager
Copy link
Collaborator

✅ Jenkins job (on internal Jenkins) succeeded

✅ Build succeeded
✅ Packages:

✅ API Diff (from stable)
✅ API Diff (from PR only) (no change)
✅ Generator Diff (no change)
ℹ️ Test run skipped: Not running tests here because they're run on public Jenkins.

@monojenkins

This comment has been minimized.

Sign in to view
@xamarin-release-manager

This comment has been minimized.

Sign in to view
@xamarin-release-manager
Copy link
Collaborator

❌ Build was (probably) aborted

🔥 Jenkins job (on internal Jenkins) failed in stage(s) 'Checkout' 🔥 : hudson.AbortException: There is already a newer build in progress (#5)

@monojenkins
Copy link
Collaborator

Build success
ℹ️ Skipped execution

@monojenkins

This comment has been minimized.

Sign in to view
@xamarin-release-manager

This comment has been minimized.

Sign in to view
@xamarin-release-manager

This comment has been minimized.

Sign in to view
@monojenkins

This comment has been minimized.

Sign in to view
@xamarin-release-manager

This comment has been minimized.

Sign in to view
@rolfbjarne
[sampletester] Bump nuget timeout to 5 minutes.
b9c03c5 
The TodoREST.iOS test seems to use a lot of packages, and it can apparently
take a while to restore them, so give a few more minutes.
@rolfbjarne
Improvements.
be8f77b
@rolfbjarne rolfbjarne removed the do-not-merge Do not merge this pull request label Dec 19, 2019
@dotnet dotnet deleted a comment from monojenkins Dec 19, 2019
@rolfbjarne
Copy link
Member Author

This PR is ready for review now.

@xamarin-release-manager
Copy link
Collaborator

✅ Jenkins job (on internal Jenkins) succeeded

✅ Build succeeded
✅ Packages:

✅ API Diff (from stable)
✅ API Diff (from PR only) (no change)
✅ Generator Diff (no change)
ℹ️ Test run skipped: Not running tests here because they're run on public Jenkins.

dalexsoto
dalexsoto approved these changes Dec 19, 2019
View reviewed changes
Copy link
Member

@dalexsoto dalexsoto left a comment

Choose a reason for hiding this comment

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

LGTM! This is great!

spouliot
spouliot reviewed Dec 19, 2019
View reviewed changes
Copy link
Contributor

@spouliot spouliot left a comment

Choose a reason for hiding this comment

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

In the data.xml it seems that sample-repository="https://github.com/xamarin/xamarin-macios" is incorrect, should not it be (one of) the sample repo ?

@spouliot
Copy link
Contributor 

xamarin-macios-hash="1b847a12bb3bf3c005db18236dd423d461c20e55" 
sample-repository="https://github.com/xamarin/xamarin-macios" 
sample-hash="1b847a12bb3bf3c005db18236dd423d461c20e55">

if also looks like the hash of the sample is the same as macios hash (one is wrong)

stephen-hawley
stephen-hawley reviewed Dec 19, 2019
View reviewed changes
tools/devops/push-performance-data.sh
{
echo '<?xml version="1.0" encoding="utf-8" standalone="yes"?>'
echo '<performance version="1.0">'
find "$job" -name '*perfdata*.xml' -print0 | xargs -0 -n 1 tail -n +2 | grep -F -v -e '<performance>' -e '</performance>'
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

spouliot
spouliot suggested changes Dec 19, 2019
View reviewed changes
Copy link
Contributor

@spouliot spouliot left a comment

Choose a reason for hiding this comment

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

pending sample-repository / sample-hash questions in produced data

tests/sampletester/ProcessHelper.cs
@@ -89,7 +95,7 @@ public static void BuildSolution (string solution, string platform, string confi

foreach (var sln in solutions) {
nuget_args [1] = sln; // replacing here
AssertRunProcess ("nuget", nuget_args.ToArray (), TimeSpan.FromMinutes (2), Configuration.SampleRootDirectory, environment_variables, "nuget restore");
AssertRunProcess ("nuget", nuget_args.ToArray (), TimeSpan.FromMinutes (5), Configuration.SampleRootDirectory, environment_variables, "nuget restore");
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 I though that the 2 minutes was an internal timeout of nuget :)
https://github.com/xamarin/maccore/issues/1616

@monojenkins
Copy link
Collaborator

Build success
ℹ️ Skipped execution

@xamarin-release-manager
Copy link
Collaborator

✅ Jenkins job (on internal Jenkins) succeeded

✅ Build succeeded
✅ Packages:

✅ API Diff (from stable)
✅ API Diff (from PR only) (no change)
✅ Generator Diff (no change)
ℹ️ Test run skipped: Not running tests here because they're run on public Jenkins.

@monojenkins
Copy link
Collaborator

Build success
ℹ️ Skipped execution

@xamarin-release-manager
Copy link
Collaborator

✅ Jenkins job (on internal Jenkins) succeeded

✅ Build succeeded
✅ Packages:

✅ API Diff (from stable)
✅ API Diff (from PR only) (no change)
✅ Generator Diff (no change)
ℹ️ Test run skipped: Not running tests here because they're run on public Jenkins.

spouliot
spouliot approved these changes Dec 20, 2019
View reviewed changes
Copy link
Contributor

@spouliot spouliot left a comment

Choose a reason for hiding this comment

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

attributes looks good now :)
https://raw.githubusercontent.com/xamarin/xamarin-macios-data/master/perf-data/samples/pull-request/7096bee643af6fbe55cc7abf2b89602d0c7df24b/data-307a3e1d-f0f2-5a3d-98c5-8c16e3f24b09.xml

Great job! The data should prove quite helpful :)

@rolfbjarne rolfbjarne merged commit 0ec82e7 into master Dec 20, 2019
spouliot pushed a commit to spouliot/xamarin-macios that referenced this pull request Feb 3, 2020
@rolfbjarne
[sample-tester] Collect app size and build duration statistics. (dotn…
1d0dd15 
…et#7602)

* [sample-tester] Collect app size and build duration statistics.

* Always publish performance data.

* [mmp] Add support for MMP_ENV_OPTIONS to mirror mtouch's MTOUCH_ENV_OPTIONS.

* [sample-tester] Make mmp/mtouch show timing information, and get the diagnostic msbuild log.

* Collect task and target info.

* Add more perf data and a baseline test.

* Redo the base test a bit.

* More tweaks.

* More tweaks.

* Huh?

* Debug stuff.

* diagnostics.

* Better diagnostics.

* cleanup

* tmp

* Use a separate stage to push data.

* Next attempt.

* Unique artifact names.

* Make the right name unique.

* More progress

* Improvements

* Don't need new maccore.

* cleanup

* Remove debug spew.

* Realign stuff.

* duh

* More path fixes.

* zippity zip.

* Show publishing errors.

* dependencies

* Not fake results anymore.

* Tweaks

* Merge xml files

* [sampletester] Bump nuget timeout to 5 minutes.

The TodoREST.iOS test seems to use a lot of packages, and it can apparently
take a while to restore them, so give a few more minutes.

* Improvements.

* Fix glob.

* [sampletester] Fix fetching of sample repository and hash.

* [sampletester] Centralize verbosity for mtouch and mmp.
@rolfbjarne rolfbjarne deleted the sample-test-statistics branch May 5, 2020 12:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dalexsoto dalexsoto dalexsoto approved these changes

@spouliot spouliot spouliot approved these changes

@chamons chamons chamons approved these changes

@VincentDondain VincentDondain VincentDondain approved these changes

@stephen-hawley stephen-hawley stephen-hawley approved these changes

@emaf emaf Awaiting requested review from emaf

@jstedfast jstedfast Awaiting requested review from jstedfast

@kzu kzu Awaiting requested review from kzu

Assignees
No one assigned
Labels
build-package Build (and create package) on internal Jenkins. Apply 'run-internal-tests' to run tests too. not-notes-worthy Ignore for release notes run-sample-tests Run the sample tests skip-all-tests Skip all the tests skip-public-jenkins Completely skip execution in the public Jenkins instance
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@rolfbjarne @monojenkins @xamarin-release-manager @spouliot @dalexsoto @chamons @VincentDondain @stephen-hawley