Expose whether a VtValue is locally stored

[sdawson-nvidia]

VtValues can make a decision to store the type they wrap around either locally as part of the VtValue's storage, or externally on the heap. When copying VtValues (as far as I understand), this means they either behave as a copy of a locally stored value or effectively a shared pointer to a heap allocation.

To determine what type a VtValue is holding to then perform more operations (particularly in cases like a user primvar where we don't already know the expected type), we have to either lookup a typeid or call IsHolding for the types we support until we get a true value, which is not the cheapest operation. To avoid repeating this process often, we were extracting the pointer to the data (using UncheckedGet once we determined the type, and either taking the cdata() of a VtArray or the address of a non-array value), and relevant type information for us (int vs. float, number of components, etc.) for later usage, and storing it in a struct alongside the VtValue.

This fails however if we copy the struct and its VtValue and the data is locally stored, since the data pointer is tied to the actual instance of the VtValue. We were using the function _IsLocallyStored to determine when to refresh the pointer since it was still public in a former version of USD, but when we upgraded our USD version this function became private. For now our most correct option is to never assume the data pointer will remain the same, and refresh the pointer each time, but this requires going through the type lookup in order to eventually call UncheckedGet. For now we're hoping that this doesn't end up being a bottleneck, and if it does our plan is to copy the logic of _IsLocallyStored to make the same determination, but obviously that exposes us to the risk of this logic changing in future USD verisons.

To me, it makes sense to allow users to get more information about the type and storage of a VtValue by exposing things similar to the _TypeInfoImpl's static bools, such as a public IsLocallyStored() function. Alternatively, we could also solve for our use case if there was a cheaper method to get the raw pointer to the data stored by the VtValue, so we could extract type information once and then use this function to always refresh the data pointer, but I'm not sure if this is really philosophically aligned with the VtValue interface.

Please let me know if there's more information that would help explain our use case, or an alternative method of avoiding repeated type checks that we're unaware of.

[jesschimein]

Filed as internal issue #USD-10645

(This is an automated message. See here for more information.)

[spiffmon]

Hi @sdawson-nvidia , can you please have a look at VtVisitValue, which @gitamohr developed a couple years ago based on similar observations and feedback from our own developers about the big if-then-else cascades. We have found it to dramatically clean up our code, while simultaneously providing noticeable dinks in some of our imaging performance tests.

For some reason it is not getting picked up in our doxygen build, and I don't think we made alot of noise about it when it landed, but we're going to make sure it's much more salient in Vt's documentation, as we'd like for it to be the philosophically-aligned solution to the kinds of problems you and many other developers face with VtValue extraction - please let us know if it does not address your problems, and feel free to close this issue if you find it does!

[gitamohr]

In addition to VtVisitValue, as Spiff suggests, which I think could help you remove some of the long if (IsHolding<T>()) { chains, I wanted to ask if storing a function pointer in the struct that returns the address of the held value would work? I might be misunderstanding so pls lmk. I was imagining something like this:

struct ValueInfo
{
    VtValue val;
    int numComponents;
    // etc..
    void const *(*getData)(VtValue const &);
};

ValueInfo
PopulateValueInfo(VtValue const &val)
{
    ValueInfo info;
    info.val = val;
    if (val.IsHolding<VtFloatArray>()) {
        val.numComponents = 1;
        val.getData = [](VtValue const &v) {
            return static_cast<void const *>(&v.UncheckedGet<VtFloatArray>());
        };
    }
    // etc..
}

[sdawson-nvidia]

@spiffmon Neat. VtVisitValue will go a long way to reducing the costliness of refetching the data, and so will likely address this use case. That being said, is there a specific reason you're reticent to expose information like local storage?

@gitamohr That does look like another option. Thank you!

[spitzak]

We are using VtVisitValue for this and it works well.

That said, it would be nice if Pixar added some convenience functions implemented using VtVisitValue, to go along with VtValue::GetArraySize(), since everybody is writing their own versions. Something like this:

VtValue::GetVectorSize() returns size of vector (ie 4 for GfVec4f), returns 1 for scalar, 0 for non-numeric
VtValue::GetArraySize() change to return 1 for non-array
VtValue::GetDouble(unsigned index) return indexed number cast to double. For arrays index is array index * vector size + field index.
VtValue::SetDouble(unsigned index, double) change the indexed number (copy on write for shared pointers).
VtValue::GetUnsigned(index) similar for other data types

[spiffmon]

@spiffmon Neat. VtVisitValue will go a long way to reducing the costliness of refetching the data, and so will likely address this use case. That being said, is there a specific reason you're reticent to expose information like local storage?

The more implementation details we expose in public API, the more our hands will be tied in making future optimizations and enhancements.

@spitzak , I'd argue it's a feature that GetArraySize() returns zero for non-arrays, as that definitively tells you it's not an array - you can't access the value as an array if it's a scalar... but I guess you could argue IsArrayValued would help you out there.
For the other enhancements:

• For the Get/Set pairs, we're either talking an API explosion, or a templated method(s). If the latter, that also means increasing the size of the function table of the internal holder class, which we are protective of, so there'd need to be a pretty compelling case?
• Also for the Set.. are you proposing it would mutate the held VtArray, or create and return a new VtValue with a different VtArray that has been updated from the original? Mutating the held one would be problematic, I think.