RendererAlgo::objectSamples : Output single samples for static objects

[danieldresser-ie]

I've got a task on the task board with a fairly vague description: "Detect static motion in InstancerCapsule.". While doing some initial investigation on this, I was assuming this related the fact that it is possible to trigger Arnold warnings "discarded X duplicate deformation keys". During some quick initial testing, I was unable to find anything specific to InstancerCapsule, but what I did notice is that if I set up an object that does not change with time, but has a hash that does change, then I get that warning ( whether or not is rendered as an InstancerCapsule ).

It seemed like this stuff might be easiest to discuss with some code to look at. I've added two more tests to RendererAlgoTest, one which documents existing behaviour ( outputting a single sample for the range of times if all samples have the same Gaffer hash ), and one that fails with the existing code ( rendering samples which don't have the same Gaffer hash, but are actually identical ).

I've also included a simple possible fix: after checking the Gaffer hashes, also check the Cortex hashes of the objects to see if they're identical - since the Cortex hashes should actually include all data in the object, unlike the Gaffer hashes which just include the parameters contributing to the object's creation.

Possible concerns with this approach:

• evaluating the Cortex hashes is somewhat expensive ( possibly more expensive than just doing an actual compare of the full data for an object? ). I had thought that we were storing hashes for objects that are expensive to hash, so that we wouldn't be needing to repeat this work if the hash is also required by something like IECoreArnold::Renderer::InstanceCache, but maybe I recalled wrong.
• we should probably consider whether we are masking legitimate problems with this approach. If the upstream network is producing different hashes, and requiring us to fully evaluate the geo before we can figure out that it isn't changing, then that is definitely hurting performance. Should we output our own warning in this case? That would kind of conflict with the goal of getting rid of the warning that's been showing up in Arnold.

Anyway, this does appear to fix the issue of getting "duplicate deformation keys" messages from Arnold. Switching to returning a single sample from this method feels a bit dangerous ... except that we're already doing this when the input hashes the same, so any clients of this method must already be able to deal with receiving single samples even if multiple were requested.

[danieldresser-ie]

In our conversation this morning, John's conclusion was that we weren't worrying about masking legitimate problems - the Arnold message is often being triggered in situations where there isn't really an appropriate user action to fix it, so we should just silently do our best to handle this data.

John did request that I try to come up with some performance test to assess whether this could have an impact ( we are storing the hashes, I was just looking in the wrong place, so the impact shouldn't be that large, but changing when the hash happens affects whether it gets computed multiple times by multiple threads ). I tested with a very heavy plane, with an animated transform and a FreezeTransform, instanced many times ( with each time offset shared between multiple instances ) - this seems like it should be pretty worst case for this code - there is no actual duplication to save on, but we are doing the hashing.

I had a difficult time seeing any performance difference ... if anything, this PR might have performed a percent or two better than the previous code, but this seemed unlikely to be statistically significant. But when I tried instrumenting Cortex to count how many times we actually did compute a duplicate hash ... turns out this PR is actually significantly reducing the number of duplicate hashes occurring ( there's a bunch of randomness, but I was seeing 90 - 110 duplicate hashes in this test before this PR, and 40 - 60 after ). This presumably means that the existing hashing that is occurring in the instance cache is more likely to occur at the same time on multiple threads. I don't think there's any reason to think that this benefit would be true in general - duplicate hashing is very dependent on things happening to occur simultaneously, and this change just altered the timing in a way that happened to be beneficial in my specific test scene.

The thing that perhaps is comforting about this test is that even in a scene built to try and emphasize hashing time, a 50% reduction in hashing had a negligible impact on scene expansion time. This suggests that it's unlikely the hashing is going to have a big impact ... in combination with the observation that the this new time to hash things is probably not any more likely to occur simultaneously than the existing hashing ... maybe that means this is reasonable?

I've added a Changes entry in case you think this is ready for merging.

[johnhaddon]

I tested with a very heavy plane,

I'm not sure the hashing of large arrays is worst-case for this - that gets cached. What doesn't get cached is the Primitive-level hash, so I think worst-case might actually be lots and lots of small primitive variables, where we have to combine the hash for them all every time. Could you do a quick test to see if that's an issue at all, and if it's not, we can put this to bed.

[danieldresser-ie]

I was more concerned about multi-threading resulting in duplication of the actual object var hashing, since that seemed like it could be significant and unpredictable.

The Primitive level hash does have a cost, but it's much more in line with the sort of hashing we're doing all the time in Gaffer. Measuring on 250 000 instances, not encapsulated so that objectSamples gets called for each one, I'm seeing scene expansion times of 8.5s with this PR, vs 6.6s without, with 1000 prim vars. Reducing that to 200 prim vars gives me 5.45 s vs 5.18s. So, 25% cost for 1000 prim vars, 5% cost for 200 prim vars. Looks like it can be approximated as a cost of a 0.025% increase in scene expansion time per prim var on the average object being exported.