Skip to content

make yoga threadsafe #852

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
tcdat96 wants to merge 2 commits into from
Closed

make yoga threadsafe #852

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cdc7512
switch from global variables to function parameters in `Yoga`
Mar 4, 2019
a9238c4
Merge branch 'master' into yoga_threadsafe
tcdat96 May 30, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions yoga/YGLayout.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,12 @@ struct YGLayout {
bool doesLegacyStretchFlagAffectsLayout : 1;
bool hadOverflow : 1;

uint32_t computedFlexBasisGeneration = 0;
uint8_t computedFlexBasisGeneration = 0;
Copy link
Contributor

@davidaurelio davidaurelio Jan 28, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What was the motivation for this change? Due to alignment requirements, this won’t even save memory without reordering data members.

Copy link

@jpap jpap Jan 30, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The motivation is to save heap: there are many bits left over from alignment of the bitfields above it, so you could combine computedFlexBasisGeneration with it. I suspect you can get away with just one bit, so long as the generation counter is the same width (or is truncated with a logical AND mask when the generation is incremented).

YGFloatOptional computedFlexBasis = {};

// Instead of recomputing the entire layout every single time, we cache some
// information to break early when nothing changed
uint32_t generationCount = 0;
uint8_t generationCount = 0;
YGDirection lastOwnerDirection = (YGDirection) -1;

uint32_t nextCachedMeasurementsIndex = 0;
Expand Down
2 changes: 1 addition & 1 deletion yoga/YGNode.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ void YGNode::setLayoutPosition(float position, int index) {
}

void YGNode::setLayoutComputedFlexBasisGeneration(
uint32_t computedFlexBasisGeneration) {
uint8_t computedFlexBasisGeneration) {
layout_.computedFlexBasisGeneration = computedFlexBasisGeneration;
}

Expand Down
2 changes: 1 addition & 1 deletion yoga/YGNode.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ struct YGNode {
void setLayoutLastOwnerDirection(YGDirection direction);
void setLayoutComputedFlexBasis(const YGFloatOptional computedFlexBasis);
void setLayoutComputedFlexBasisGeneration(
uint32_t computedFlexBasisGeneration);
uint8_t computedFlexBasisGeneration);
void setLayoutMeasuredDimension(float measuredDimension, int index);
void setLayoutHadOverflow(bool hadOverflow);
void setLayoutDimension(float dimension, int index);
Expand Down
121 changes: 81 additions & 40 deletions yoga/Yoga.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -935,7 +935,7 @@ bool YGNodeLayoutGetDidLegacyStretchFlagAffectLayout(const YGNodeRef node) {
return node->getLayout().doesLegacyStretchFlagAffectsLayout;
}

uint32_t gCurrentGenerationCount = 0;
uint8_t gCurrentGenerationCount = 0;

bool YGLayoutNodeInternal(
const YGNodeRef node,
Expand All @@ -950,7 +950,9 @@ bool YGLayoutNodeInternal(
const char* reason,
const YGConfigRef config,
YGMarkerLayoutData& layoutMarkerData,
void* const layoutContext);
void* const layoutContext,
const uint32_t depth,
const uint8_t generationCount);

#ifdef DEBUG
static void YGNodePrintInternal(
Expand Down Expand Up @@ -1194,7 +1196,9 @@ static void YGNodeComputeFlexBasisForChild(
const YGDirection direction,
const YGConfigRef config,
YGMarkerLayoutData& layoutMarkerData,
void* const layoutContext) {
void* const layoutContext,
const uint32_t depth,
const uint8_t generationCount) {
const YGFlexDirection mainAxis =
YGResolveFlexDirection(node->getStyle().flexDirection(), direction);
const bool isMainAxisRow = YGFlexDirectionIsRow(mainAxis);
Expand All @@ -1217,8 +1221,7 @@ static void YGNodeComputeFlexBasisForChild(
if (child->getLayout().computedFlexBasis.isUndefined() ||
(YGConfigIsExperimentalFeatureEnabled(
child->getConfig(), YGExperimentalFeatureWebFlexBasis) &&
child->getLayout().computedFlexBasisGeneration !=
gCurrentGenerationCount)) {
child->getLayout().computedFlexBasisGeneration != generationCount)) {
const YGFloatOptional paddingAndBorder = YGFloatOptional(
YGNodePaddingAndBorderForAxis(child, mainAxis, ownerWidth));
child->setLayoutComputedFlexBasis(
Expand Down Expand Up @@ -1369,13 +1372,16 @@ static void YGNodeComputeFlexBasisForChild(
"measure",
config,
layoutMarkerData,
layoutContext);
layoutContext,
depth,
generationCount
);

child->setLayoutComputedFlexBasis(YGFloatOptional(YGFloatMax(
child->getLayout().measuredDimensions[dim[mainAxis]],
YGNodePaddingAndBorderForAxis(child, mainAxis, ownerWidth))));
}
child->setLayoutComputedFlexBasisGeneration(gCurrentGenerationCount);
child->setLayoutComputedFlexBasisGeneration(generationCount);
}

static void YGNodeAbsoluteLayoutChild(
Expand All @@ -1387,7 +1393,9 @@ static void YGNodeAbsoluteLayoutChild(
const YGDirection direction,
const YGConfigRef config,
YGMarkerLayoutData& layoutMarkerData,
void* const layoutContext) {
void* const layoutContext,
const uint32_t depth,
const uint8_t generationCount) {
const YGFlexDirection mainAxis =
YGResolveFlexDirection(node->getStyle().flexDirection(), direction);
const YGFlexDirection crossAxis = YGFlexDirectionCross(mainAxis, direction);
Expand Down Expand Up @@ -1493,7 +1501,9 @@ static void YGNodeAbsoluteLayoutChild(
"abs-measure",
config,
layoutMarkerData,
layoutContext);
layoutContext,
depth,
generationCount);
childWidth = child->getLayout().measuredDimensions[YGDimensionWidth] +
child->getMarginForAxis(YGFlexDirectionRow, width).unwrap();
childHeight = child->getLayout().measuredDimensions[YGDimensionHeight] +
Expand All @@ -1513,7 +1523,9 @@ static void YGNodeAbsoluteLayoutChild(
"abs-layout",
config,
layoutMarkerData,
layoutContext);
layoutContext,
depth,
generationCount);

if (child->isTrailingPosDefined(mainAxis) &&
!child->isLeadingPositionDefined(mainAxis)) {
Expand Down Expand Up @@ -1817,7 +1829,9 @@ static float YGNodeComputeFlexBasisForChildren(
const YGConfigRef config,
bool performLayout,
YGMarkerLayoutData& layoutMarkerData,
void* const layoutContext) {
void* const layoutContext,
const uint32_t depth,
const uint8_t generationCount) {
float totalOuterFlexBasis = 0.0f;
YGNodeRef singleFlexChild = nullptr;
YGVector children = node->getChildren();
Expand Down Expand Up @@ -1868,7 +1882,7 @@ static float YGNodeComputeFlexBasisForChildren(
continue;
}
if (child == singleFlexChild) {
child->setLayoutComputedFlexBasisGeneration(gCurrentGenerationCount);
child->setLayoutComputedFlexBasisGeneration(generationCount);
child->setLayoutComputedFlexBasis(YGFloatOptional(0));
} else {
YGNodeComputeFlexBasisForChild(
Expand All @@ -1883,7 +1897,9 @@ static float YGNodeComputeFlexBasisForChildren(
direction,
config,
layoutMarkerData,
layoutContext);
layoutContext,
depth,
generationCount);
}

totalOuterFlexBasis +=
Expand Down Expand Up @@ -1997,7 +2013,9 @@ static float YGDistributeFreeSpaceSecondPass(
const bool performLayout,
const YGConfigRef config,
YGMarkerLayoutData& layoutMarkerData,
void* const layoutContext) {
void* const layoutContext,
const uint32_t depth,
const uint8_t generationCount) {
float childFlexBasis = 0;
float flexShrinkScaledFactor = 0;
float flexGrowFactor = 0;
Expand Down Expand Up @@ -2163,7 +2181,9 @@ static float YGDistributeFreeSpaceSecondPass(
"flex",
config,
layoutMarkerData,
layoutContext);
layoutContext,
depth,
generationCount);
node->setLayoutHadOverflow(
node->getLayout().hadOverflow |
currentRelativeChild->getLayout().hadOverflow);
Expand Down Expand Up @@ -2294,7 +2314,9 @@ static void YGResolveFlexibleLength(
const bool performLayout,
const YGConfigRef config,
YGMarkerLayoutData& layoutMarkerData,
void* const layoutContext) {
void* const layoutContext,
const uint32_t depth,
const uint8_t generationCount) {
const float originalFreeSpace = collectedFlexItemsValues.remainingFreeSpace;
// First pass: detect the flex items whose min/max constraints trigger
YGDistributeFreeSpaceFirstPass(
Expand All @@ -2320,7 +2342,9 @@ static void YGResolveFlexibleLength(
performLayout,
config,
layoutMarkerData,
layoutContext);
layoutContext,
depth,
generationCount);

collectedFlexItemsValues.remainingFreeSpace =
originalFreeSpace - distributedFreeSpace;
Expand Down Expand Up @@ -2620,7 +2644,9 @@ static void YGNodelayoutImpl(
const bool performLayout,
const YGConfigRef config,
YGMarkerLayoutData& layoutMarkerData,
void* const layoutContext) {
void* const layoutContext,
const uint32_t depth,
const uint8_t generationCount) {
YGAssertWithNode(
node,
YGFloatIsUndefined(availableWidth)
Expand Down Expand Up @@ -2800,7 +2826,9 @@ static void YGNodelayoutImpl(
config,
performLayout,
layoutMarkerData,
layoutContext);
layoutContext,
depth,
generationCount);

const bool flexBasisOverflows = measureModeMainDim == YGMeasureModeUndefined
? false
Expand Down Expand Up @@ -2905,7 +2933,9 @@ static void YGNodelayoutImpl(
performLayout,
config,
layoutMarkerData,
layoutContext);
layoutContext,
depth,
generationCount);
}

node->setLayoutHadOverflow(
Expand Down Expand Up @@ -3079,7 +3109,9 @@ static void YGNodelayoutImpl(
"stretch",
config,
layoutMarkerData,
layoutContext);
layoutContext,
depth,
generationCount);
}
} else {
const float remainingCrossDim = containerCrossAxis -
Expand Down Expand Up @@ -3287,7 +3319,9 @@ static void YGNodelayoutImpl(
"multiline-stretch",
config,
layoutMarkerData,
layoutContext);
layoutContext,
depth,
generationCount);
}
}
break;
Expand Down Expand Up @@ -3427,7 +3461,9 @@ static void YGNodelayoutImpl(
direction,
config,
layoutMarkerData,
layoutContext);
layoutContext,
depth,
generationCount);
}

// STEP 11: SETTING TRAILING POSITIONS FOR CHILDREN
Expand Down Expand Up @@ -3455,7 +3491,6 @@ static void YGNodelayoutImpl(
}
}

uint32_t gDepth = 0;
bool gPrintChanges = false;
bool gPrintSkips = false;

Expand Down Expand Up @@ -3661,16 +3696,18 @@ bool YGLayoutNodeInternal(
const char* reason,
const YGConfigRef config,
YGMarkerLayoutData& layoutMarkerData,
void* const layoutContext) {
void* const layoutContext,
uint32_t depth,
const uint8_t generationCount) {
#ifdef YG_ENABLE_EVENTS
Event::publish<Event::NodeLayout>(node);
#endif
YGLayout* layout = &node->getLayout();

gDepth++;
depth++;

const bool needToVisitNode =
(node->isDirty() && layout->generationCount != gCurrentGenerationCount) ||
(node->isDirty() && layout->generationCount != generationCount) ||
layout->lastOwnerDirection != ownerDirection;

if (needToVisitNode) {
Expand Down Expand Up @@ -3760,8 +3797,7 @@ bool YGLayoutNodeInternal(

if (!needToVisitNode && cachedResults != nullptr) {
layout->measuredDimensions[YGDimensionWidth] = cachedResults->computedWidth;
layout->measuredDimensions[YGDimensionHeight] =
cachedResults->computedHeight;
layout->measuredDimensions[YGDimensionHeight] = cachedResults->computedHeight;

(performLayout ? layoutMarkerData.cachedLayouts
: layoutMarkerData.cachedMeasures) += 1;
Expand All @@ -3772,8 +3808,8 @@ bool YGLayoutNodeInternal(
YGLogLevelVerbose,
nullptr,
"%s%d.{[skipped] ",
YGSpacer(gDepth),
gDepth);
YGSpacer(depth),
depth);
node->print(layoutContext);
Log::log(
node,
Expand All @@ -3795,8 +3831,8 @@ bool YGLayoutNodeInternal(
YGLogLevelVerbose,
nullptr,
"%s%d.{%s",
YGSpacer(gDepth),
gDepth,
YGSpacer(depth),
depth,
needToVisitNode ? "*" : "");
node->print(layoutContext);
Log::log(
Expand All @@ -3823,16 +3859,18 @@ bool YGLayoutNodeInternal(
performLayout,
config,
layoutMarkerData,
layoutContext);
layoutContext,
depth,
generationCount);

if (gPrintChanges) {
Log::log(
node,
YGLogLevelVerbose,
nullptr,
"%s%d.}%s",
YGSpacer(gDepth),
gDepth,
YGSpacer(depth),
depth,
needToVisitNode ? "*" : "");
node->print(layoutContext);
Log::log(
Expand Down Expand Up @@ -3896,8 +3934,7 @@ bool YGLayoutNodeInternal(
node->setDirty(false);
}

gDepth--;
layout->generationCount = gCurrentGenerationCount;
layout->generationCount = generationCount;
return (needToVisitNode || cachedResults == nullptr);
}

Expand Down Expand Up @@ -4070,7 +4107,9 @@ void YGNodeCalculateLayoutWithContext(
"initial",
node->getConfig(),
marker->data,
layoutContext)) {
layoutContext,
0, // tree root
gCurrentGenerationCount)) {
node->setPosition(
node->getLayout().direction, ownerWidth, ownerHeight, ownerWidth);
YGRoundToPixelGrid(node, node->getConfig()->pointScaleFactor, 0.0f, 0.0f);
Expand Down Expand Up @@ -4123,7 +4162,9 @@ void YGNodeCalculateLayoutWithContext(
"initial",
nodeWithoutLegacyFlag->getConfig(),
layoutMarkerData,
layoutContext)) {
layoutContext,
0, // tree root
gCurrentGenerationCount)) {
nodeWithoutLegacyFlag->setPosition(
nodeWithoutLegacyFlag->getLayout().direction,
ownerWidth,
Expand Down