Search Unity

Bug ScriptableBuildPipeline - PrefabPackedIdentifiers generate duplicate ids

Discussion in 'Asset Bundles' started by OndrejP, Jul 18, 2021.

  1. OndrejP

    OndrejP

    Joined:
    Jul 19, 2017
    Posts:
    176
    I've tried using ScriptableBuildPipeline (CompatibilityBuildPipeline) instead of the old BuildPipeline to build my asset bundles.

    I've found that PrefabPackedIdentifiers generates duplicate IDs for some of my objects. During runtime the Unity Player crashed when loading the bundle with TransformChangeDispatch::QueueTransformChangeIfHasChanged (probably not important).

    Build error + stack trace in attachment. When I set ContiguousBundles to false, everything worked (it switches to Unity5PackedIdentifiers).

    When looking at the code of both PrefabPackedIdentifiers and Unity5PackedIdentifiers it looks really stupid. The way simple data structures like GUID and long are converted to string or casted to objects and then written through some weird reflection into hash stream and then hash is obtained as byte array of 16-bytes, just to throw away half of it. WOW, small wonder it even works.

    Can you please look at it and fix those two classes which calculate the identifiers to make it work properly? What about using Hash128 to combine GUID, LocalFileID and FileType?
     

    Attached Files:

    • err.txt
      File size:
      13.3 KB
      Views:
      22
    Last edited: Jul 18, 2021
  2. Ryanc_unity

    Ryanc_unity

    Unity Technologies

    Joined:
    Jul 22, 2015
    Posts:
    326
    This will be improved in SBP 1.19.2
    Collisions can still occur sadly, but in 1.19.2 we've exposed a few options in Preferences to help combat the issue on a per project basis.
     
    OndrejP likes this.
  3. OndrejP

    OndrejP

    Joined:
    Jul 19, 2017
    Posts:
    176
    Thanks, I'm okay with Hash collisions as long as it's lower probability than being killed by an asteroid :)
     
  4. Ryanc_unity

    Ryanc_unity

    Unity Technologies

    Joined:
    Jul 22, 2015
    Posts:
    326
    The Unity5PackedIdentifiers method, believe it or not is what is used by native C++ exactly ever since Unity 5.x, and while not perfect, is fairly collision free.

    PrefabPackedIdentifiers on the other hand, ya, we were over optimistic by our implementation of that one. The up coming changes I've tested heavily at 1,000,000 assets and 1,000,000 objects to be collision free, additionally I've tested the quality of the results to ensure the "clustering" quality we wanted to achieve with that for increased load times is still reliable as it achieves <1% assets per cluster reuse. While 0% reuse is perfect, it comes with a collision trade off.

    In addition we also added the ability to inject a seed into the hashing, so if you do get a collision on an unreleased project, you can generate and save a random seed to get around the collision.

    Finally we now properly catch collision errors earlier at generation time and fail the build as a result.
     
    Last edited: Aug 5, 2021
    OndrejP likes this.
  5. OndrejP

    OndrejP

    Joined:
    Jul 19, 2017
    Posts:
    176
    Sounds great! Does it improve load performance even when using LoadAllAssets()?
    From what I found in docs, it seems that LoadAllAssets is fastest, I'd expect it loads sequentially, thus eliminating the need to have assets "in-order" (therefore does not benefit from PrefabPackedIdentifiers), is that correct?
     
  6. Ryanc_unity

    Ryanc_unity

    Unity Technologies

    Joined:
    Jul 22, 2015
    Posts:
    326
    Correct, LoadAllAssets loads everything start to finish linearly in order, limited to no seeking required.
    Loading single assets still loads "in-order" to remove the need for back seeking, but if there are a lot of assets (such as prefabs or fbx models) in a single bundle, that forward seeking can still be heavy, so PrefabPackedIdentifiers clusters objects in the bundle together based on which source asset they came from to reduce the amount of forward seeking to load a single asset.
     
    OndrejP likes this.
  7. Ryanc_unity

    Ryanc_unity

    Unity Technologies

    Joined:
    Jul 22, 2015
    Posts:
    326
    Here's an example of one such large bundle from a customer. Each unique color represents 1KB of loadable data for a given loadable asset.

    Unity5PackedIdentifiers:


    PrefabPackedIdentifiers:


    LoadAllAssets didn't care that it was all mixed up as it just loads start to finish.
     
    OndrejP likes this.
  8. OndrejP

    OndrejP

    Joined:
    Jul 19, 2017
    Posts:
    176
    Thanks for clarification, that's interesting!
     
unityunity