CesiumGS / cesium-unity

Bringing the 3D geospatial ecosystem to Unity
https://cesium.com/platform/cesium-for-unity/
Apache License 2.0
347 stars 83 forks source link

Update to 3D Tiles 1.1 metadata #380

Closed j9liu closed 7 months ago

j9liu commented 10 months ago

We should update the metadata system to handle the 3D Tiles 1.1 extensions, instead of the deprecated EXT_feature_metadata. Currently, we have a single CesiumMetadata component that enables retrieving features from a particular triangle in a tile, which is really different from Cesium for Unreal. I think Cesium for Unreal's API is much more flexible, so I've been trying to replicate it in Unity, down to the same classes and structs.

My idea was to attach CesiumModelMetadata components to the game objects representing glTF models, and CesiumPrimitiveFeatures to the game objects representing primitives. @kring expressed concern about performance. I also worried this might not be scalable, because I experienced some lag while developing. However... my laptop was not plugged in, so that skewed my perception of things 😅

I tested the new implementation more seriously today, and found that the difference isn't actually that noticeable. I did two views on the 05_CesiumMetadata level; first, the initial starting view, and second, a horizon shot containing many more buildings.

View 1 View 2
image image

Using the performance profiler, I looked at the Update.ScriptRunBehaviorUpdate look under the function overview, since it tracked how long each component's Update took in a frame.

View 1: Single Component Multi Component
old image
View 2: Single Component Multi Component
Old2 image

The multi-component implementation ranges from 1.2-1.9ms, while the single component implementation ranges from 1.3-2.08ms. So for some reason, the new implementation seems fine, if not a little faster. But I'm not sure why! I'm testing using two different copies of the samples, one straight from main and one for development Not sure if that makes a difference.

@kring I'm happy to do more testing if you're not convinced, but I'm inclined to stick with my multi-component approach for now.

csciguy8 commented 10 months ago

What categories under Update.ScriptRunBehaviorUpdate should be affected most?

Actually, if you're pretty far along, maybe make a draft PR, and I can just look myself.

j9liu commented 10 months ago

I believe the concern was that having numerous components in the scene would impact performance because N tiles would have N components, whereas the current implementation just uses one CesiumMetadata component.

However, I can't see a real performance difference with the profiler. I was looking under the Update loop because I thought, if the N MonoBehaviours had any performance impact, it'd be because you have N more components to update. But I'm not seeing them appear in the hierarchy. I looked in that particular region because you can see updates of the other components listed (e.g. Cesium3DTileset).I don't see entries for any of the components I added.

In any case, I pushed my progress to the upgrade-feature-metadata branch, if you're interested.

kring commented 10 months ago

I wouldn't expect much change to average frame time. The things to watch out for are:

  1. Increased memory usage. Both from the extra MonoBehaviours themselves and from garbage collection being delayed by the need to run finalizers.
  2. Increased time spent in garbage collection. Probably not every frame, but in "spikes".

But it may be a total non-issue, for two reasons:

  1. Unity already uses tons of native code, so the extra cost of finalization might already be baked in everywhere.
  2. When we are done with a tile, we Dispose all of its MonoBehaviours. Hopefully. At least, we should. I think we do. And as long as we do that, finalization is suppressed, and so the things to watch out for above won't happen at all.
j9liu commented 10 months ago

@kring My bad, I completely misremembered!

I did a test where I spawned into the metadata sample level and just spun around the camera a couple times, trying to prompt tiles to constantly load in and out. These are the memory graphs between the two, with the marker on the largest GC spike I could find.

Single Component Multiple Component
Graph profilerOld profilerCurrent
Maximum Total Used Memory 3.38GB 3.36 GB
Largest GC spike 287.1KB 269.5KB

So it looks like my branch is comparable to what's on main. I'm not sure why it seems slightly better, tbh. I could try to do another pair of tests with pre-programmed camera movement so the setups are more identical.

kring commented 10 months ago

Ok seems promising, thanks for checking! One more quick thing to check, though: turn off all the other CPU usage categories and see if you see any larger spikes in the GarbageCollector category. That should tell us if the GC is spending more time finalizing. Maybe you could already tell that this wasn't a concern, but it's hard to tell from the screenshots.

Just to clarify, the number of GC bytes allocated isn't a huge concern by itself. The bigger concern is the need for finalization keeping native memory live longer. I think that would show up in the "Other" or "Untracked" categories, though, and there doesn't seem to be any increase there, either.

j9liu commented 10 months ago

One more quick thing to check, though: turn off all the other CPU usage categories and see if you see any larger spikes in the GarbageCollector category.

So... I am running into a problem where the GarbageCollector category is always empty in the profiler? This happens for both cases I'm testing.

image

I searched for "GC" and "Garbage" in the CPU function hierarchy, but I don't see any numbers where the garbage is actually being collected.

image image

Have you seen this before? Or is there something wrong with my setup? 😅