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

[RC1] Could not load type 'System.Utf8String' from assembly 'System.Utf8String.Experimental #41521

Closed
adamsitnik opened this issue Aug 28, 2020 · 18 comments

Comments

@adamsitnik
Copy link
Member

Problem

Unable to use Utf8String in 5.0 RC1 SDK (5.0.0-rc.1.20423.8), it works fine in 6.0 alpha (6.0.100-alpha.1.20428.8)

Unhandled exception. System.TypeLoadException: Could not load type 'System.Utf8String' from assembly 'System.Utf8String.Experimental, Version=5.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51'.
   at repro.Program.Main()

Repro:

The following code fails when using 5.0.0-rc.1.20423.8 SDK:

using System;

namespace repro
{
    class Program
    {
        static void Main() => Console.WriteLine(new Utf8String("Hello World!"));
    }
}
<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net5.0</TargetFramework>
  </PropertyGroup>
  <ItemGroup>
    <PackageReference Include="System.Utf8String.Experimental" Version="5.0.0-preview.8.20352.3" />
  </ItemGroup>
</Project>
<?xml version="1.0" encoding="utf-8"?>
<configuration>
  <packageSources>
    <clear />
    <add key="nuget" value="https://api.nuget.org/v3/index.json" />
    <add key="dotnet5" value="https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet5/nuget/v3/index.json" />
  </packageSources>
</configuration>

The problem is currently blocking performance repo from switching to release/5.0.1xx channel for downloading net5.0 SDK (so far we were using master channel but it's time to switch to 5.0 as master contains now 6.0 bits): dotnet/performance#1484

@GrabYourPitchforks @eerhardt is there any chance that someone could take a look at this?

/cc @ooooolivia

@adamsitnik adamsitnik added area-System.Runtime blocking Marks issues that we want to fast track in order to unblock other important work labels Aug 28, 2020
@adamsitnik adamsitnik added this to the 5.0.0 milestone Aug 28, 2020
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label Aug 28, 2020
@AndyAyersMS
Copy link
Member

I hit this too.

Also ran into issues with the deprecation of binary formatter, the suggested suppression via SYSLIB0011 did not work and the only way I could get things to build was to delete the files that had binary formatter tests.

@layomia
Copy link
Contributor

layomia commented Aug 28, 2020

cc @Anipik, @ericstj I think this has to do with #40960 where experimental types were pulled out of RC. As expected, the type are available in master/.NET 6. Is the expectation that consumers can use these experimental types in release candidates/builds?

@ericstj
Copy link
Member

ericstj commented Aug 28, 2020

I believe that is expected. We don’t want experimental types in release builds. They are experimental, not to be released.

@jkotas
Copy link
Member

jkotas commented Aug 28, 2020

The performance repo should not be building these tests by default, or maybe even not have them at all.

@joperezr
Copy link
Member

Closing this issue as per the discussion above this is expected.

@adamsitnik
Copy link
Member Author

I believe that is expected. We don’t want experimental types in release builds. They are experimental, not to be released.

But how can we then use this type in net5.0 apps? Even for "experiments"?

The performance repo should not be building these tests by default, or maybe even not have them at all.

they have been useful to measure the ARM64 vectorization improvements (cc @carlossanlop @pgovind) and even exposed some regressions (#41388)

I am aware of the fact that we won't include them just to make me or a few other Utf8String enthusiasts happy, but if we want to break the "chicken-egg-circle" we somehow need to allow devs to experiment with that type. If I was a dev who wanted to experiment with this type to check possible performance gains from switching to Utf8 then my user experience would be really bad. A compilation error that would tell me to use 5.0 preview SDK would be much better

@adamsitnik
Copy link
Member Author

Closing this issue as per the discussion above this is expected.

It might be expected, but the problem still exists and a runtime error that does not contain any explanation is just a very bad user experience. I am reopening so at least a proper error message is added.

@adamsitnik adamsitnik reopened this Aug 28, 2020
@jkotas
Copy link
Member

jkotas commented Aug 28, 2020

But how can we then use this type in net5.0 apps? Even for "experiments"?

You cannot. The plan for this is to move experiment like this to dotnet/runtimelab and allow you to publish self-contained apps if you want to play with experimental features like Utf8String.

@jkotas
Copy link
Member

jkotas commented Aug 28, 2020

dotnet/runtimelab#3

@ericstj
Copy link
Member

ericstj commented Aug 28, 2020

@adamsitnik why would you expect something different than a TypeLoadException here? The type is missing. We aren’t shipping this package. We definitely don’t want to ship some random prerelease version in corelib... that was a mess with Span in 2.0. I guess we could have a dummy copy of the type that throws an exception in constructor pointing folks to an aka.ms link. Does that meet 5.0 bar at this point?

As @jkotas mentioned the POR is for this to come from rubtimelab moving forward. That avoids these types of problems.

@jkotas
Copy link
Member

jkotas commented Aug 28, 2020

even exposed some regressions (#41388)

The workhorse method is used by regular UTF8Encoding (UTF8Encoding.GetCharCountFast -> Utf8Utility.GetPointerToFirstInvalidByte -> ASCIIUtility.GetIndexOfFirstNonAsciiByte). This regression should have been caught by UTF8Encoding benchmarks too. If it was not, it suggests we may have a UTF8Encoding coverage hole.

@adamsitnik
Copy link
Member Author

why would you expect something different than a TypeLoadException here?

I am installing a NuGet package, writing code that compiles perfectly fine, and then I am trying to run it. Once I get TypeLoadException my first thought is that I am missing some dependency (a .dll file) or there is a .dll version mismatch. Since it worked just a few days ago, I am very confused.

The type is missing. We aren’t shipping this package.

If the type has been removed from net5.0 I should get a compiler error that indicates that.

We definitely don’t want to ship some random prerelease version in corelib..

This is perfectly fine, but please be clear about this. Give me an error that says it and I won't be creating new issues and wondering whether SDK installer is broken or something was removed, but not 100% removed.

If it was not, it suggests we may have a UTF8Encoding coverage hole.

I've sent a PR to address that: dotnet/performance#1491

@adamsitnik adamsitnik removed blocking Marks issues that we want to fast track in order to unblock other important work untriaged New issue has not been triaged by the area owner labels Aug 28, 2020
@adamsitnik adamsitnik removed this from the 5.0.0 milestone Aug 28, 2020
@jkotas
Copy link
Member

jkotas commented Aug 28, 2020

f the type has been removed from net5.0 I should get a compiler error that indicates that.

You would get the compiler if you were not referencing old preview version of the experimental package: https://github.com/dotnet/performance/blob/master/src/benchmarks/micro/MicroBenchmarks.csproj#L42

@adamsitnik
Copy link
Member Author

You would get the compiler if you were not referencing old preview version of the experimental package:

One of the first things I've tried was to update to latest version. According to VS update (including prereleases, using dotnet5 NuGet feed) it is not possible:

obraz

@jkotas
Copy link
Member

jkotas commented Aug 28, 2020

That is the compile build error. The IDE is trying to be helpful by not allowing an action that would lead to a build error.

@adamsitnik
Copy link
Member Author

As far as I know if a newer package existed VS would show a possible update and fail after hitting the update button. Then I could go to the output tab and see the actual error. But it did not and this is why I created this issue

@AndyAyersMS
Copy link
Member

FWIW I worked around the UTF8String issues by using an older runtime as the BDN host runtime, eg

dotnet run -c Release -f netcoreapp3.1 --  --filter *.FannkuchRedux* --runtimes netcoreapp3.1 netcoreapp5.0 --join

Seems odd somehow that this works for older runtimes (including net461) and not for 5.0; from what I can tell the sources do get compiled...

@jkotas
Copy link
Member

jkotas commented Aug 28, 2020

if a newer package existed VS would show a possible update and fail after hitting the update button

The situation here is that the old preview package does not work anymore and there is no replacement. NuGet has no way to communicate that the package is dead-ended without replacement today. It is a feature request on NuGet - not the first time we have a situation like this.

Seems odd somehow that this works for older runtimes

It is side-effect of the creative way this package is constructed.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants