[Entities 1.3.9] StableTypeHash Broken

Unity Engine
, , ,
1

Looks like TypeHash.CalculateStableTypeHash(typeof(SomeComponent)) now returns a different value than TypeManager.GetTypeInfo<SomeComponent>().StableTypeHash

1.3.8 works fine. Has anyone run into the same problem?

1 Like
2

I’ve reproduced this problem locally. A workaround for now is to add the scripting define DISABLE_TYPEMANAGER_ILPP to your project. This will have a domain reload performance regression, but the behaviour will return to normal until we address it.

2 Likes
3

Just FYI,

There are multiple other bugs that we ran into with the current StaticTypeRegistry ILPP, here are a couple more we found while upgrading from 1.2.x to 1.3.9:

  1. The ILPP throws for systems with [DisableAutoCreation], no default constructor and static variables, this is because the ILPP assumes that systems have a default constructor in InjectCreateSystem, we reported the bug here as long with a fix: https://support.unity.com/hc/en-us/requests/2085131
  2. InjectGetSystemAttributes generates invalid IL in some circumstances, somehow this is not a problem for Mono but it causes IL2CPP to throw at compilation.
    We haven’t reported this one yet, but this is because that method is missing a whole lot of checks and doesn’t handle enum field assignments properly (this can be fixed in InjectLoadFromCustomArgument by adding a case for enums in arguments of type MetadataType.Class), here’s a simple system that will cause IL2CPP to throw:
using Unity.Entities;

[WorldSystemFilter(WorldSystemFilterFlags.Default, ChildDefaultFilterFlags = WorldSystemFilterFlags.Default)]
public partial class SystemCausingIL2CPPToFail : SystemBase
{
    protected override void OnUpdate() {}
}
  1. The ILPP (just like the TypeManager to some extent) always includes all type names regardless of the current configuration (release or debug), this is because of this hardcoded value in StaticTypeRegistryPostProcessor: IsReleaseConfig = false;//!EntitiesILPostProcessors.Defines.Contains("DEBUG");
    But this is also an issue with the TypeManager, in our fork, we have made changes to the TypeManager to exclude type names in retail builds when DOTS_DISABLE_DEBUG_NAMES is present as well as prevent a call to EntityNameStorage.Initialize.
    With a thousand systems and around 8000 types in the type manager, this saved us multiple megabytes which we’ll gladly take considering we’re not using that data
  2. Finally, in our case, the methods generated by both InjectCreateSystem and InjectGetSystemAttributes are massive 15000 instructions behemoth as we just have so many systems in our project, I haven’t looked at the generated assembly from IL2CPP nor did I profile any of that so I’m not sure how effectively this is optimized by the cpp compiler, but I’m afraid this might not scale well when too many systems are present as I don’t think that many branches against that many typeof() calls is going to perform well :eyes:

For the time being we will likely disable the ILPP until this is sorted but maybe this should be the default until this has been properly tested?

4

A bunch of us reported a lot of the il generation bugs directly to the developer responsible for working on this on discord

Fixes for this and all the other typemanager issues reported in discord that I’m aware of (about 5 total) have landed, so it should be fixed in the next release

And as of 3 days ago all the know issues were fixed, pending next release. I’m sure they’ll see this post if there were any missed.

4 Likes
5

Hey, @mcapdegelle. I’ve created a bug report for this issue. Here’s the Issue Tracker link for this case: https://issuetracker.unity3d.com/product/unity/issues/guid/ECSB-1507. Cheers!

1 Like
6

Hey, since we already have this thread, not sure if this one is known as well, but we just found another bug related to Nullable fields in IComponent that leads to memory corruption.

If you have system type fields like System.Nullable or System.Guid in an IComponent, the ILPP is not able to properly get their size which eventually leads to memory stomping when iterating on data, here is a small repro case that shows the bug:

using Unity.Entities;
using UnityEngine;
using static Unity.Entities.SystemAPI;

public partial struct MemoryCorruption : ISystem
{
    struct BrokenComponent : IComponentData
    {
        public uint a;
        public uint b;
        public uint c;
        public byte? d;
        public byte? e;
    }

    unsafe void OnCreate(ref SystemState state)
    {
        ref readonly var typeInfo = ref TypeManager.GetTypeInfo<BrokenComponent>();

        EntityArchetype entityArchetype = state.EntityManager.CreateArchetype(stackalloc ComponentType[] { ComponentType.ReadWrite<BrokenComponent>() });
        Entity a = state.EntityManager.CreateEntity(entityArchetype);
        Entity b = state.EntityManager.CreateEntity(entityArchetype);
        Entity c = state.EntityManager.CreateEntity(entityArchetype);

        // prints: sizeof=16 | ElementSize=28 | SizeInChunk=28
        Debug.Log($"sizeof={sizeof(BrokenComponent)} | ElementSize={typeInfo.ElementSize} | SizeInChunk={typeInfo.SizeInChunk}");

        BrokenComponent newValue = new BrokenComponent
        {
            b = 0xFFFFFFFF,
            d = 0xFF,
            e = 0xFF
        };

        foreach (var foo in Query<RefRW<BrokenComponent>>())
        {
            foo.ValueRW = newValue;
        }

        Debug.Assert(GetComponent<BrokenComponent>(a).Equals(newValue));
        Debug.Assert(GetComponent<BrokenComponent>(b).Equals(newValue)); // Fails, memory of b has been stomped
        Debug.Assert(GetComponent<BrokenComponent>(c).Equals(newValue)); // Fails, memory of c has been stomped
    }
}

This same code works fine in Entities 1.3.8.
Also for the record, adding the scripting define DISABLE_TYPEMANAGER_ILPP leads to a compile error: Library\PackageCache\com.unity.entities@1.3.9\Unity.Entities\Types\TypeManager.cs(2634,53): error CS0103: The name 'typesToReprocess' does not exist in the current context

7

Since I was told that StableTypeHash won’t be stable over entities versions and some things in the future will be breaking even if they attempt to go back to the old behaviour, (assemblies are hashed and these currently changed from mscorlib to netstandard for any primitive) I went ahead and made my own StableTypeManager. It was actually quite simple. Feel free to use it.

8

Just don’t put the word Safety in your field :sweat_smile:

9

Thanks for the reminder. :smiley: It’s changed now and not that rigid.