Skip to content

Commit

Permalink
Ignore adjustment heuristics if size in weighCallSite is negative
Browse files Browse the repository at this point in the history 
The estimated code size of methods can be negative; this is to indicate
that the method is a good candidate for inlinling. However, the logic in
weighCallSite, which contains several heuristics that adjust the size of
a candidate method, used a variable of type uint32_t.

The first issue is that the code that compares the size of a method to
the inlining budget results in promoting the inlining budge value from a
signed 32-bit integer to an unsigned 32-bit integer, because the
variable that represents the size of the candidate method is an unsigned
32-bit integer. This means that any negative size, which is supposed to
represent a good candidate method, will actually cause the inliner to
make the determination that it is a very bad candidate method.

The second issue is that even if if the comparison was fixed to cast the
size variable to be a signed 32-bit integer, the heuristics prior to
that point that adjust the size can result in an overflow that now
leaves the size variable with a very large signed positive number, which
again, causes the inliner to make the determination that the candidate
method is very bad for inlining.

This commit fixes both issues by
1. Changing the size variable to be of type int32_t
2. Only allowing the heuristics to adjust the size if it is positive.

Signed-off-by: Irwin D'Souza <[email protected]>
  • Loading branch information
@dsouzai
dsouzai committed Jan 22, 2024
1 parent ae8282b commit b065415
Showing 1 changed file with 51 additions and 43 deletions.
94 changes: 51 additions & 43 deletions runtime/compiler/optimizer/InlinerTempForJ9.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3354,7 +3354,7 @@ void TR_MultipleCallTargetInliner::weighCallSite( TR_CallStack * callStack , TR_

for (int32_t k = 0; k < callsite->numTargets(); k++)
{
uint32_t size = 0;
int32_t size = 0;

TR_EstimateCodeSize::raiiWrapper ecsWrapper(this, tracer(), _maxRecursiveCallByteCodeSizeEstimate);
TR_EstimateCodeSize *ecs = ecsWrapper.getCodeEstimator();
Expand Down Expand Up @@ -3452,7 +3452,7 @@ void TR_MultipleCallTargetInliner::weighCallSite( TR_CallStack * callStack , TR_
heuristicTrace(tracer(),"WeighCallSite: For Target %p node %p signature %s, estimation returned a size of %d",
calltarget,calltarget->_myCallSite->_callNode,tracer()->traceSignature(calltarget),size);

if (currentBlockHasExceptionSuccessors && ecs->aggressivelyInlineThrows())
if (currentBlockHasExceptionSuccessors && ecs->aggressivelyInlineThrows() && size > 0)
{
_maxRecursiveCallByteCodeSizeEstimate >>= 3;
size >>= 3;
Expand All @@ -3469,7 +3469,7 @@ void TR_MultipleCallTargetInliner::weighCallSite( TR_CallStack * callStack , TR_
(size > _maxRecursiveCallByteCodeSizeEstimate/2))
possiblyVeryHotLargeCallee = true;

if (calltarget->_calleeSymbol->isSynchronised())
if (calltarget->_calleeSymbol->isSynchronised() && size > 0)
{
size >>= 1; // could help gvp
heuristicTrace(tracer(),"Setting size to %d because call is Synchronized",size);
Expand All @@ -3481,13 +3481,13 @@ void TR_MultipleCallTargetInliner::weighCallSite( TR_CallStack * callStack , TR_
wouldBenefitFromInlining = true;
}

if (strstr(calltarget->_calleeSymbol->signature(trMemory()),"BigDecimal.add("))
if (strstr(calltarget->_calleeSymbol->signature(trMemory()),"BigDecimal.add(") && size > 0)
{
size >>=2;
heuristicTrace(tracer(),"Setting size to %d because call is BigDecimal.add",size);
}

if (isHot(comp()))
if (isHot(comp()) && size > 0)
{
TR_ResolvedMethod *m = calltarget->_calleeSymbol->getResolvedMethod();
const char *sig = "toString";
Expand Down Expand Up @@ -3587,53 +3587,61 @@ void TR_MultipleCallTargetInliner::weighCallSite( TR_CallStack * callStack , TR_
if (comp()->trace(OMR::inlining))
heuristicTrace(tracer(),"WeighCallSite: Considering shrinking call %p with frequency %d\n", callsite->_callNode, frequency2);

bool largeCompiledCallee = !comp()->getOption(TR_InlineVeryLargeCompiledMethods) &&
isLargeCompiledMethod(calltarget->_calleeMethod, size, frequency2);
if (largeCompiledCallee)
{
size = size*TR::Options::_inlinerVeryLargeCompiledMethodAdjustFactor;
}
else if (frequency2 > borderFrequency)
if (size > 0)
{
float factor = (float)(maxFrequency-frequency2)/(float)maxFrequency;
factor = std::max(factor, 0.4f);
bool largeCompiledCallee = !comp()->getOption(TR_InlineVeryLargeCompiledMethods) &&
isLargeCompiledMethod(calltarget->_calleeMethod, size, frequency2);
if (largeCompiledCallee)
{
size = size*TR::Options::_inlinerVeryLargeCompiledMethodAdjustFactor;
}
else if (frequency2 > borderFrequency)
{
float factor = (float)(maxFrequency-frequency2)/(float)maxFrequency;
factor = std::max(factor, 0.4f);

float avgMethodSize = (float)size/(float)ecs->getNumOfEstimatedCalls();
float numCallsFactor = (float)(avgMethodSize)/110.0f;
float avgMethodSize = (float)size/(float)ecs->getNumOfEstimatedCalls();
float numCallsFactor = (float)(avgMethodSize)/110.0f;

numCallsFactor = std::max(factor, 0.1f);
numCallsFactor = std::max(factor, 0.1f);

if (size > 100)
{
size = (int)((float)size * factor * numCallsFactor);
if (size < 100) size = 100;
if (size > 100)
{
size = (int)((float)size * factor * numCallsFactor);
if (size < 100) size = 100;
}
else
{
size = (int)((float)size * factor * numCallsFactor);
}
if (comp()->trace(OMR::inlining))
heuristicTrace(tracer(), "WeighCallSite: Adjusted call-graph size for call node %p, from %d to %d\n", callsite->_callNode, origSize, size);
}
else
else if ((frequency2 > 0) && (frequency2 < veryColdBorderFrequency)) // luke-warm block
{
size = (int)((float)size * factor * numCallsFactor);
float factor = (float)frequency2 / (float)maxFrequency;
//factor = std::max(factor, 0.1f);
size = (int)((float)size / (factor*factor)); // make the size look bigger to inline less
if (comp()->trace(OMR::inlining))
heuristicTrace(tracer(), "WeighCallSite: Adjusted call-graph size for call node %p, from %d to %d\n", callsite->_callNode, origSize, size);
}
if (comp()->trace(OMR::inlining))
heuristicTrace(tracer(), "WeighCallSite: Adjusted call-graph size for call node %p, from %d to %d\n", callsite->_callNode, origSize, size);
}
else if ((frequency2 > 0) && (frequency2 < veryColdBorderFrequency)) // luke-warm block
{
float factor = (float)frequency2 / (float)maxFrequency;
//factor = std::max(factor, 0.1f);
size = (int)((float)size / (factor*factor)); // make the size look bigger to inline less
if (comp()->trace(OMR::inlining))
heuristicTrace(tracer(), "WeighCallSite: Adjusted call-graph size for call node %p, from %d to %d\n", callsite->_callNode, origSize, size);
}
else if ((frequency2 >= 0) && (frequency2 < coldBorderFrequency)) // very cold block
{
//to avoid division by zero crash. Semantically freqs of 0 and 1 should be pretty close given maxFrequency of 10K
int adjFrequency2 = frequency2 ? frequency2 : 1;
float factor = (float)adjFrequency2 / (float)maxFrequency;
//factor = std::max(factor, 0.1f);
size = (int)((float)size / factor);
else if ((frequency2 >= 0) && (frequency2 < coldBorderFrequency)) // very cold block
{
//to avoid division by zero crash. Semantically freqs of 0 and 1 should be pretty close given maxFrequency of 10K
int adjFrequency2 = frequency2 ? frequency2 : 1;
float factor = (float)adjFrequency2 / (float)maxFrequency;
//factor = std::max(factor, 0.1f);
size = (int)((float)size / factor);


if (comp()->trace(OMR::inlining))
heuristicTrace(tracer(),"WeighCallSite: Adjusted call-graph size for call node %p, from %d to %d\n", callsite->_callNode, origSize, size);
if (comp()->trace(OMR::inlining))
heuristicTrace(tracer(),"WeighCallSite: Adjusted call-graph size for call node %p, from %d to %d\n", callsite->_callNode, origSize, size);
}
else
{
if (comp()->trace(OMR::inlining))
heuristicTrace(tracer(),"WeighCallSite: Not adjusted call-graph size for call node %p, size %d\n", callsite->_callNode, origSize);
}
}
else
{
Expand Down

0 comments on commit b065415

Please sign in to comment.