if (FAABBTimeSliceCVars::bUseTimeSliceMillisecondBudget)
{
// Checking for platform time every time is expensive as its cost adds up fast, so only do it between a configured amount of elements.
// This means we might overshoot the budget, but on the other hand we will keep most of the time doing actual work.
CurrentDataElementsCopiedSinceLastCheck++;
if (CurrentDataElementsCopiedSinceLastCheck > FAABBTimeSliceCVars::MinDataChunkToProcessBetweenTimeChecks)
{
CurrentDataElementsCopiedSinceLastCheck = 0;
return bCanContinueCopy;
}
const double ElapseTime = FPlatformTime::Seconds() - StartSliceTimeStamp;
if (!FMath::IsNearlyZero(StartSliceTimeStamp) && ElapseTime > FAABBTimeSliceCVars::MaxProcessingTimePerSliceSeconds)
{
bCanContinueCopy = false;
}
}
it seems that above code (taken from AABBTree.h) is not doing what it says it does. As I understand it should do time check every MinDataChunkToProcessBetweenTimeChecks, but instead it is doing it always except every MinDataChunkToProcessBetweenTimeChecks. So condition is inverted. Should not it be something like this:
if (FAABBTimeSliceCVars::bUseTimeSliceMillisecondBudget)
{
CurrentDataElementsCopiedSinceLastCheck++;
if (CurrentDataElementsCopiedSinceLastCheck > FAABBTimeSliceCVars::MinDataChunkToProcessBetweenTimeChecks)
{
CurrentDataElementsCopiedSinceLastCheck = 0;
}
else
{
return bCanContinueCopy;
}
const double ElapseTime = FPlatformTime::Seconds() - StartSliceTimeStamp;
if (!FMath::IsNearlyZero(StartSliceTimeStamp) && ElapseTime > FAABBTimeSliceCVars::MaxProcessingTimePerSliceSeconds)
{
bCanContinueCopy = false;
}
}
Another thing is with check below. What I observe it that StartSliceTimeStamp is not always set to current time at the beginning of a progress task (instead it is having value from the last frame and I see it in the profiler by inserting bookmarks and calculating time), so there are cases when this condition will fail and so StartSliceTimeStamp will be set to 0 and we will return while we actually should not.
if (!CanContinueCopyingDataCallback(MaximumBytesToCopy, SizeCopiedSoFar))
{
// For data copy, the time stamp is set from the setup phase, and it is copied from the tree we are copying from.
// Therefore if we reach this point with no time available left, just reset it so the next frame can start counting from scratch
StartSliceTimeStamp = 0.0;
return;
}