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

Calculatation of sync on properties, relatively expensive #19535

Open
anvilvapre opened this issue Jul 17, 2021 · 3 comments
Open

Calculatation of sync on properties, relatively expensive #19535

anvilvapre opened this issue Jul 17, 2021 · 3 comments

Comments

@anvilvapre
Copy link
Contributor

Version: Bleed.

0.6% of all time spent in calculating sync hashes - across all game sync objects - is spent in calculating the sync hash for property QuantizedFacings of BodyOrientation objects. This is the most time spent across all sync objects.

Request:

Pleas optimize this.

Possible solution:

  • Remove Sync attribute.
  • Determine the impact/overhaed of the Lazy<> evaluation.
  • Determine if Sync cannot better be on a cached value - rather than the evaluation the 'expensive' get method.

In general it could be a good idea to look at all Sync properties to determine the expense of their Get methods. Ideally you don't want to calculate Syncs over properties - but rather fields. The sync will cause the possibly expensive calculation to be re executed once more.

Perhaps the game could log in debug mode those properties - that have a sync attribute - that take over x millisecond to calculate. Or better a code analyzer tool could inspect on the complicity of a get-er. This could be an indicator of Sync calling expensive get-ers.

@pchote
Copy link
Member

pchote commented Jul 17, 2021

This doesn't make sense to me: the Lazy is a memoization primitive: it only evaluates the function once, and then returns the cached value for every subsequent call. The first time might be slow, but every other call should be essentially free.

Is all the overhead due to the sync machinery having to deal with a property rather than a field?

@anvilvapre
Copy link
Contributor Author

It seems the ~0.7% is typical for each/more properties. So then it is not related to the/this specific properties.

These traits show during an early stage of replay. On the shell map they different. See the three examples below. The other traits do not show up when I profile at max frequency. Seem they do not get sampled.

   - 26.05% instance int32 [OpenRA.Game] OpenRA.World::SyncHash()[MinOptJitted]                                                                                  ▒
      - 14.97% instance int32 [OpenRA.Game] OpenRA.Actor+SyncHash::Hash()[MinOptJitted]                                                                          ▒
           0.68% int32 [OpenRA.Mods.Common] dynamicClass::hash_BodyOrientation(object)[MinOptJitted]                                                             ▒
           0.50% int32 [OpenRA.Mods.Common] dynamicClass::hash_AppearsOnRadar(object)[MinOptJitted]                                                              ▒
        5.67% instance bool [OpenRA.Game] OpenRA.TraitDictionary+TraitContainer`1+<Actors>d__14[System.__Canon]::MoveNext()[MinOptJitted]                        ▒
        0.59% instance bool [System.Collections] System.Collections.Generic.SortedSet`1+Enumerator[System.Collections.Generic.KeyValuePair`2[System.UInt32,Syste
-   39.73%     8.71%  dotnet           [JIT] tid 29122             [.] instance int32 [OpenRA.Game] OpenRA.World::SyncHash()[MinOptJitted]                       ▒
   - 31.02% instance int32 [OpenRA.Game] OpenRA.World::SyncHash()[MinOptJitted]                                                                                  ▒
      - 18.78% instance int32 [OpenRA.Game] OpenRA.Actor+SyncHash::Hash()[MinOptJitted]                                                                          ▒
           0.74% int32 [OpenRA.Mods.Common] dynamicClass::hash_Mobile(object)[MinOptJitted]                                                                      ▒
           0.72% int32 [OpenRA.Mods.Common] dynamicClass::hash_DamageMultiplier(object)[MinOptJitted]                                                            ▒
           0.71% instance bool [OpenRA.Mods.Common] OpenRA.Mods.Common.Traits.ConditionalTrait`1[System.__Canon]::get_IsTraitDisabled()[MinOptJitted]            ▒
           0.69% int32 [OpenRA.Mods.Common] dynamicClass::hash_AppearsOnRadar(object)[MinOptJitted]                                                              ▒
           0.64% int32 [OpenRA.Mods.Common] dynamicClass::hash_BodyOrientation(object)[MinOptJitted]                                                             ▒
           0.55% int32 [OpenRA.Mods.Common] dynamicClass::hash_WithDecoration(object)[MinOptJitted]                                                              ▒
      - 6.95% instance bool [OpenRA.Game] OpenRA.TraitDictionary+TraitContainer`1+<Actors>d__14[System.__Canon]::MoveNext()[MinOptJitted]                        ▒
           0.63% instance !0 [System.Private.CoreLib] System.Collections.Generic.List`1[System.__Canon]::get_Item(int32)[OptimizedTier1]                         ▒
        0.66% instance bool [System.Collections] System.Collections.Generic.SortedSet`1+Enumerator[System.Collections.Generic.KeyValuePair`2[System.UInt32,Syste

Later in game, 6 bots fighting:

-   38.07%     8.19%  dotnet           [JIT] tid 29122             [.] instance int32 [OpenRA.Game] OpenRA.World::SyncHash()[MinOptJitted]                       ▒
   - 29.87% instance int32 [OpenRA.Game] OpenRA.World::SyncHash()[MinOptJitted]                                                                                  ▒
      - 17.50% instance int32 [OpenRA.Game] OpenRA.Actor+SyncHash::Hash()[MinOptJitted]                                                                          ▒
           0.69% int32 [OpenRA.Mods.Common] dynamicClass::hash_Mobile(object)[MinOptJitted]                                                                      ▒
           0.65% instance bool [OpenRA.Mods.Common] OpenRA.Mods.Common.Traits.ConditionalTrait`1[System.__Canon]::get_IsTraitDisabled()[MinOptJitted]            ▒
           0.53% int32 [OpenRA.Mods.Common] dynamicClass::hash_BodyOrientation(object)[MinOptJitted]                                                             ▒
           0.51% int32 [OpenRA.Mods.Common] dynamicClass::hash_AppearsOnRadar(object)[MinOptJitted]                                                              ▒
      - 6.89% instance bool [OpenRA.Game] OpenRA.TraitDictionary+TraitContainer`1+<Actors>d__14[System.__Canon]::MoveNext()[MinOptJitted]                        ▒
           0.72% instance !0 [System.Private.CoreLib] System.Collections.Generic.List`1[System.__Canon]::get_Item(int32)[OptimizedTier1]                         ▒
        0.55% instance bool [System.Collections] System.Collections.Generic.SortedSet`1+Enumerator[System.Collections.Generic.KeyValuePair`2[System.UInt32,Syst

@anvilvapre anvilvapre changed the title BodyOrientation.QuantizedFacings expensive sync Calculatation of sync on properties, relatively expensive Jul 23, 2021
@Mailaender
Copy link
Member

Remove Sync attribute.

or caching it doesn't sound like a good idea. It is the only way to get details from the state during desync investigation.

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

No branches or pull requests

4 participants