experiments/STT-tensorflow
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Do not change scope_id in AllocatorAttributes::Merge when it matches other.

Browse Source 
Before this change, if attr1 and attr2 had the same scope_id value,
attr1.Merge(attr2) would result in attr1.scope_id being set to 0.  This change
makes Merge do nothing to scope_id if it is the same, which seems like the
natural behavior of "merging" the same values.

This change also introduces a CHECK to ensure that, when merging, both scope
ids are not non-zero.

PiperOrigin-RevId: 255506545
This commit is contained in:
Ayush Dubey 2019-06-27 16:53:28 -07:00 committed by TensorFlower Gardener
parent 50987b3224
commit a29691ddf2
2 changed files with 53 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
tensorflow/core/framework/allocator.h
View File

@ -288,9 +288,13 @@ struct AllocatorAttributes {
bool gpu_compatible() const { return value & (0x1 << 2); }
void Merge(AllocatorAttributes other) {
value |= other.value;
scope_id = (scope_id > 0 && other.scope_id == 0)
? scope_id
: ((scope_id == 0) ? other.scope_id : 0);
if (scope_id != other.scope_id) {
CHECK(scope_id == 0 || other.scope_id == 0)
<< "At least one scope_id should be zero to merge "
"AllocatorAttributes but found this.scope_id="
<< scope_id << " and other.scope_id=" << other.scope_id;
scope_id = scope_id == 0 ? other.scope_id : scope_id;
}
}
// Returns true if the fields set in *this is a subset of or equal to
// those set in other.

46
tensorflow/core/framework/allocator_test.cc
View File

@ -86,6 +86,52 @@ TEST(AllocatorAttributesTest, IsEqualOrLessRestrictiveThan) {
EXPECT_FALSE(a.IsEqualOrLessRestrictiveThan(b));
}
TEST(AllocatorAttributesTest, Merge) {
AllocatorAttributes a, b;
// Merging nic_compatible=True and nic_compatible=False results in
// nic_compatible=True.
EXPECT_EQ(a.value, 0);
EXPECT_EQ(b.value, 0);
EXPECT_FALSE(a.nic_compatible());
EXPECT_FALSE(b.nic_compatible());
b.set_nic_compatible(true);
a.Merge(b);
EXPECT_TRUE(a.nic_compatible());
EXPECT_TRUE(b.nic_compatible());
// a.Merge(b) does not change b.
EXPECT_EQ(a.scope_id, 0);
EXPECT_EQ(b.scope_id, 0);
a.scope_id = 1;
a.Merge(b);
EXPECT_EQ(a.scope_id, 1);
EXPECT_EQ(b.scope_id, 0);
// If a.scope_id=1 and b.scope_id=0, then b.Merge(a) results in b.scope_id=1.
a.scope_id = 1;
b.scope_id = 0;
b.Merge(a);
EXPECT_EQ(a.scope_id, 1);
EXPECT_EQ(b.scope_id, 1);
// If a.scope_id and b.scope_id are same, then merge leaves them unchanged.
a.scope_id = 2;
b.scope_id = 2;
a.Merge(b);
EXPECT_EQ(a.scope_id, 2);
EXPECT_EQ(b.scope_id, 2);
}
TEST(AllocatorAttributesDeathTest, MergeDifferentScopeIds) {
AllocatorAttributes a, b;
// If a.scope_id and b.scope_id are both positive but different, then
// a.Merge(b) should cause a CHECK failure.
a.scope_id = 3;
b.scope_id = 4;
EXPECT_DEATH({ a.Merge(b); }, "");
}
TEST(CPUAllocatorTest, Simple) {
EnableCPUAllocatorStats(true);
Allocator* a = cpu_allocator();