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

[Work items] Refining ref assemblies #17612

Closed
22 of 34 tasks
jcouv opened this issue Mar 7, 2017 · 6 comments
Closed
22 of 34 tasks

[Work items] Refining ref assemblies #17612

jcouv opened this issue Mar 7, 2017 · 6 comments
Assignees
Labels
Area-Compilers Concept-Determinism The issue involves our ability to support determinism in binaries and PDBs created at build time. Feature Request
Milestone

Comments

@jcouv
Copy link
Member

jcouv commented Mar 7, 2017

Ref assemblies should have the minimum amount of stuff to preserve the compile-time behavior of consumers. Some metadata should not be emitted in ref assemblies and some diagnostics should not affect emitting ref assemblies.

C# 7.1 scope

  • Integrate into various build systems
    • MSBuild and Project System
    • Domino
    • Windows build
      • Do they need to extract the MVID?
      • Figure out Windows issue with tlbexp tool
      • Sgen.exe issue was resolved (external)
  • More test ideas:
    • Invariant to changes with IVT: add more examples
    • Changing order will change refout output (determinism)
    • Constraint is maintained. "where T : struct" (in ref assembly) with "string" type (in client)
  • Remaining test plan validations
  • Measuring time savings on Roslyn
  • Document MSBuild changes
  • Add warning for ref assembly name (needs further discussion, I think we shouldn't do it)
  • Disallow EmitMetadataOnly=false with IncludePrivateMembers=false amd metadataPEStream=null
  • Add NoPIA tests in VB
  • Re-enable NoPIA
  • Non-public attributes on public APIs should not be in ref assemblies. (answer: no. All attributes must be kept. For instance, IsReadOnlyAttribute)
  • We should generate doc comment for every API that goes into the primary output. (answer: punted to later)
  • Consider emitting ref assemblies with determinism on, always (answer: we can't make the compilation deterministic, as that would prevent turning /refout by default in msbuild in the future, but we emit deterministically. Only non-determinism is with AssemblyVersion with wildcard)
  • Avoid or tolerate some diagnostics on partial methods (not applicable after all)
  • Adding private methods or types should not change ref assemblies.
  • Adding internal methods or types should change ref assemblies. (maybe if you don't have any internals-visible-to, then we do not include internal stuff)
  • csc /refonly should still produce a ref assembly if there is a semantic error in a method body.
  • Fix bug with outputting strings instead of resource strings ("CopyRefAssembly_BadDestination1")
  • Switch to ImageReader for reading the MVID (answer: using a new .mvid section instead)
  • Confirm behavior for /addmodule (answer: not supported with /refout or /refonly)
  • Add missing ReferenceAssemblyAttribute attribute (during emit phase)
  • Modifying method bodies should not change ref assemblies. (answer: done except for changes that introduce types, like anonymous types)
  • csc /refout should produce no assembly, but (at some point) may produce a ref assembly anyways. (no, we decided against partial success)
  • Fix help message (IDS_CSCHelp)

Future/extended scope:

Remaining integration issues for 15.5 are tracked by #20418
The list of future refinements was moved to #19994

References

@jcouv jcouv added Area-Compilers Concept-Determinism The issue involves our ability to support determinism in binaries and PDBs created at build time. labels Mar 7, 2017
@jcouv jcouv added this to the 3.0 milestone Mar 7, 2017
@jcouv jcouv assigned gafter and jcouv Mar 7, 2017
@jcouv jcouv changed the title Ref assemblies should not be sensitive to private changes and certain diagnostics Refine what is in reference assemblies and what diagnostics prevent generating one Mar 7, 2017
@CyrusNajmabadi
Copy link
Member

We should generate a ref assembly even if there are errors on types (emit some form of error types instead)

I have a lot of thoughts on this. Should we use this issue to track things, or should we create a new one? Thanks!

@jcouv
Copy link
Member Author

jcouv commented Mar 8, 2017

Nice. Feel free to edit the description (first post), adding details. If possible, describe as pass/fail scenarios that we can check off.

@jcouv
Copy link
Member Author

jcouv commented Mar 8, 2017

@CyrusNajmabadi I would further encourage you to suggest more tests on the open PR #17558
Those tests will be a spec of some kind (for scenarios that we'll want to track and fix with the present issue).

@jaredpar
Copy link
Member

One case to keep in mind is structs and private fields. Here is a detailed write up of the problems they cause for compilations here:

http://blog.paranoidcoding.com/2016/02/15/are-private-members-api-surface.html

I think with a little work though we can boil it down to a few easy rules to keep the surface area around structs small.

@jcouv jcouv modified the milestones: 15.3, 16.0 Mar 17, 2017
@jcouv jcouv changed the title Refine what is in reference assemblies and what diagnostics prevent generating one [Work items] Refining ref assemblies Mar 17, 2017
@jcouv
Copy link
Member Author

jcouv commented May 12, 2017

Small CSI script for manual checking of assembly timestamps, for later use:

IEnumerable<string> ListFiles(string path)
{
    foreach (var f in Directory.EnumerateFiles(path, "*.dll", SearchOption.AllDirectories)
        .Concat(Directory.EnumerateFiles(path, "*.exe", SearchOption.AllDirectories)))
    {
        if (f.Contains("obj")) continue;

        var timestamp = new FileInfo(f).LastWriteTime;
        yield return timestamp + " - " + f;
    }
}

void Print()
{
    foreach (var f in ListFiles("C:\\repos\\Client\\"))
    {
        Console.WriteLine(f);
    }
}

@jcouv jcouv modified the milestones: 15.later, 15.3 May 30, 2017
@jcouv
Copy link
Member Author

jcouv commented Jun 2, 2017

In the two scenarios I benchmarked, ref assemblies saved ~70% of the time in incremental build.

The first scenario I benchmarked is building a Roslyn IDE test project from the command-line, and making an non-public compiler change. (Anti-virus turned off, msbuild /fl /flp:v=diag /v:m /m src\EditorFeatures\CSharpTest\CSharpEditorServicesTest.csproj)

  • Clean build: 1m25s with ref assemblies, 1m18s without, (ref assembly is a bit slower)
  • Build with a comment in CodeAnalysis: 22s with ref assemblies, 1m18s without, (ref assembly is much faster)
  • Build with no change: 12s in both cases.

The second scenario was building a compiler test project, and making an non-public compiler change. (Anti-virus turned off, msbuild /fl /flp:v=diag /v:m /m src\Compilers\CSharp\Test\Emit\CSharpCompilerEmitTest.csproj)

  • Clean build: 45s without ref assemblies,
  • Build with a comment in CodeAnalysis: 9s with ref assemblies (I forgot to measure without, but I expect 45s as above)
  • Build with no change: 5s in both cases

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compilers Concept-Determinism The issue involves our ability to support determinism in binaries and PDBs created at build time. Feature Request
Projects
Status: Done Umbrellas
Development

No branches or pull requests

4 participants