Skip to content

Commit

Permalink
[JSC] Do not propagate ValueProfile and ArrayProfile in builtin code
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=261583
rdar://115529687

Reviewed by Alexey Shvayka.

Builtin JS code is used in too much different context. As a result, some of code pollute this profile (like, using ArrayStorage!),
and all subsequent code hits this polluted profile unfortunately. Because we are propagating this to UnlinkedCodeBlock, then we
will hit this pollution throughout the subsequent runs. In this patch, we stop propagating collected profiles into UnlinkedCodeBlock
when it is builtin code.

* Source/JavaScriptCore/bytecode/CodeBlock.cpp:
(JSC::CodeBlock::updateAllNonLazyValueProfilePredictionsAndCountLiveness):
(JSC::CodeBlock::updateAllArrayProfilePredictions):

Canonical link: https://commits.webkit.org/268011@main
  • Loading branch information
Constellation committed Sep 15, 2023
1 parent 90a0f95 commit 93b6b13
Showing 1 changed file with 13 additions and 4 deletions.
17 changes: 13 additions & 4 deletions Source/JavaScriptCore/bytecode/CodeBlock.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2835,6 +2835,8 @@ void CodeBlock::updateAllNonLazyValueProfilePredictionsAndCountLiveness(const Co
numberOfSamplesInProfiles = 0; // If this divided by ValueProfile::numberOfBuckets equals numberOfValueProfiles() then value profiles are full.

unsigned index = 0;
UnlinkedCodeBlock* unlinkedCodeBlock = this->unlinkedCodeBlock();
bool isBuiltinFunction = unlinkedCodeBlock->isBuiltinFunction();
forEachValueProfile([&](ValueProfile& profile, bool isArgument) {
unsigned numSamples = profile.totalNumberOfSamples();
static_assert(ValueProfile::numberOfBuckets == 1);
Expand All @@ -2843,13 +2845,17 @@ void CodeBlock::updateAllNonLazyValueProfilePredictionsAndCountLiveness(const Co
numberOfSamplesInProfiles += numSamples;
if (isArgument) {
profile.computeUpdatedPrediction(locker);
unlinkedCodeBlock()->unlinkedValueProfile(index++).update(profile);
if (!isBuiltinFunction)
unlinkedCodeBlock->unlinkedValueProfile(index).update(profile);
++index;
return;
}
if (profile.numberOfSamples() || profile.isSampledBefore())
numberOfLiveNonArgumentValueProfiles++;
profile.computeUpdatedPrediction(locker);
unlinkedCodeBlock()->unlinkedValueProfile(index++).update(profile);
if (!isBuiltinFunction)
unlinkedCodeBlock->unlinkedValueProfile(index).update(profile);
++index;
});

if (m_metadata) {
Expand Down Expand Up @@ -2886,10 +2892,13 @@ void CodeBlock::updateAllArrayProfilePredictions(const ConcurrentJSLocker& locke
return;

unsigned index = 0;

UnlinkedCodeBlock* unlinkedCodeBlock = this->unlinkedCodeBlock();
bool isBuiltinFunction = unlinkedCodeBlock->isBuiltinFunction();
auto process = [&] (ArrayProfile& profile) {
profile.computeUpdatedPrediction(locker, this);
unlinkedCodeBlock()->unlinkedArrayProfile(index++).update(profile);
if (!isBuiltinFunction)
unlinkedCodeBlock->unlinkedArrayProfile(index).update(profile);
++index;
};

m_metadata->forEach<OpGetById>([&] (auto& metadata) {
Expand Down

0 comments on commit 93b6b13

Please sign in to comment.