Skip to content

WIP : Master base sweeping simplified#455

Closed
mrvoorhe wants to merge 1 commit into
dotnet:masterfrom
Unity-Technologies:master-base-sweeping-simplified
Closed

WIP : Master base sweeping simplified#455
mrvoorhe wants to merge 1 commit into
dotnet:masterfrom
Unity-Technologies:master-base-sweeping-simplified

Conversation

@mrvoorhe
Copy link
Copy Markdown
Contributor

@mrvoorhe mrvoorhe commented Feb 1, 2019

In certain situations, such as when a static method on a type is used, the base type can be safely removed. This can then allow for the removal of overrides to abstract methods.

I tried to implement full support for this scenario, but I ran into major complexity with visibility of nested types. Rather implement all of the complex logic to figure out visibility when nested types are involved, I bail and don't try to remove base types when nested types are involved.

This didn't end up being the size saver than I originally had hoped for. An early prototype was reducing the size of mscorlib by 0.8% - 1.0% range (I don't recall exact number) for a simple Console.WriteLine("Hello World") program. However, after having to give up when nested types are involved and a rather obvious bug where I wasn't taking into account that once a type could be on the stack you need to keep the base classes, the size savings dropped to 0.25%.

That said, this can help considerably in certain scenarios. For example, if you had a program with just

        static void Main(string[] args)
        {
            //System.Xml.dll
            Console.WriteLine(typeof(System.Xml.XmlDocument));
        }

Then the size can drop dramatically.

System.Xml.dll (WIN)
  Expected: 108544
  Actual: 8192
  Diff: -100352 (-92.4528%)

That is an artificial case, but real cases could exist on a smaller scale.

  • Build up a type hierarchy table during the TypeMapStep so that a type's base classes are easily accessible during the MarkStep.

  • Sweep step will now change the base type of any class that does not need it's base to Object.

  • Add link xml tests

  • Add preserve depenendecy tests

  • Add test for ctors without bodies

  • Don't mess with attributes

  • Casting will result in type hierarchy being kept

  • Return types must have type hierarchy kept

  • Locals must have type hierarchy kept

@mrvoorhe mrvoorhe requested a review from marek-safar as a code owner February 1, 2019 18:54
@mrvoorhe
Copy link
Copy Markdown
Contributor Author

mrvoorhe commented Feb 1, 2019

This branch depends on #454. I will rebase after that PR lands to clean up commits

@mrvoorhe
Copy link
Copy Markdown
Contributor Author

mrvoorhe commented Feb 1, 2019

@marek-safar This branch isn't 100% ready yet. I'm sorting through some final issues that are appearing by running our il2cpp test suite. But it's good enough that it's ready to be reviewed for any major issues.

I'm also curious how you feel about the size savings. This does add a decent amount of complexity in exchange for a small savings.

@mrvoorhe
Copy link
Copy Markdown
Contributor Author

mrvoorhe commented Feb 1, 2019

@marek-safar
I think this failure is a pedump bug. What are your thoughts?
https://jenkins.mono-project.com/job/test-linker-pull-request/624/testReport/junit/Mono.Linker.Tests.TestCases.All/InheritanceAbstractClassTests/Mono_Linker_Tests_TestCases_All_NoKeptCtor_Visibility_Generics_NestedProtectedTypeFromBaseAsGenericConstraintOnType/

I think it's a bug because

  • peverify on windows doesn't complain
  • pedump on OSX gives the same error for the input test.exe assembly
  • That test is a case where pretty much everything is kept.

@mrvoorhe
Copy link
Copy Markdown
Contributor Author

mrvoorhe commented Feb 1, 2019

@mrvoorhe mrvoorhe mentioned this pull request Feb 4, 2019
@mrvoorhe
Copy link
Copy Markdown
Contributor Author

mrvoorhe commented Feb 4, 2019

I have a prototype of deferring instance method body marking until we know the type is instantiated. Any bodies left over at the end of the MarkStep get changed to MethodAction.ConvertToThrow. I'm not done with it yet, but for a Console.WriteLine("Hello World") program the mscorlib size goes down by 0.16%.

So although base sweeping is complex, it does appear to be the largest general purpose source of class library size savings that I have lined up.

* Build up a type hierarchy table during the TypeMapStep so that a type's base classes are easily accessible during the MarkStep.

* Sweep step will now change the base type of any class that does not need it's base to Object.

* Add link xml tests

* Add preserve depenendecy tests

* Add test for ctors without bodies

* Don't mess with attributes

* Casting will result in type hierarchy being kept

* Return types must have type hierarchy kept

* Locals must have type hierarchy kept
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant