Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(333)

Side by Side Diff: Source/platform/heap/Visitor.cpp

Issue 834673008: Extend locking of GCInfo table to include slot allocation. (Closed) Base URL: https://chromium.googlesource.com/chromium/blink.git@master
Patch Set: Adjust position of slot initialization Created 5 years, 11 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
« no previous file with comments | « Source/platform/heap/Visitor.h ('k') | no next file » | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 /* 1 /*
2 * Copyright (C) 2014 Google Inc. All rights reserved. 2 * Copyright (C) 2014 Google Inc. All rights reserved.
3 * 3 *
4 * Redistribution and use in source and binary forms, with or without 4 * Redistribution and use in source and binary forms, with or without
5 * modification, are permitted provided that the following conditions are 5 * modification, are permitted provided that the following conditions are
6 * met: 6 * met:
7 * 7 *
8 * * Redistributions of source code must retain the above copyright 8 * * Redistributions of source code must retain the above copyright
9 * notice, this list of conditions and the following disclaimer. 9 * notice, this list of conditions and the following disclaimer.
10 * * Redistributions in binary form must reproduce the above 10 * * Redistributions in binary form must reproduce the above
(...skipping 25 matching lines...) Expand all
36 36
37 namespace blink { 37 namespace blink {
38 38
39 // GCInfo indices start from 1 for heap objects, with 0 being treated 39 // GCInfo indices start from 1 for heap objects, with 0 being treated
40 // specially as the index for freelist entries and large heap objects. 40 // specially as the index for freelist entries and large heap objects.
41 int GCInfoTable::s_gcInfoIndex = 0; 41 int GCInfoTable::s_gcInfoIndex = 0;
42 42
43 size_t GCInfoTable::s_gcInfoTableSize = 0; 43 size_t GCInfoTable::s_gcInfoTableSize = 0;
44 GCInfo const** s_gcInfoTable = nullptr; 44 GCInfo const** s_gcInfoTable = nullptr;
45 45
46 size_t GCInfoTable::allocateGCInfoSlot() 46 void GCInfoTable::ensureGCInfoIndex(const GCInfo* gcInfo, size_t* gcInfoIndexSlo t)
47 { 47 {
48 // FIXME: if multiple threads attempt to allocate the initial object 48 ASSERT(gcInfo);
49 // for a given type, there'll be a write race on updating the 'static' 49 ASSERT(gcInfoIndexSlot);
50 // holding its index. Duplicate GCInfo slots might be written for the 50 // Keep a global GCInfoTable lock while allocating a new slot.
51 // object, which is benign, but verify that the index update race is 51 AtomicallyInitializedStatic(Mutex&, mutex = *new Mutex);
52 // acceptable. 52 MutexLocker locker(mutex);
53 int index = atomicIncrement(&s_gcInfoIndex); 53
54 // If more than one thread ends up allocating a slot for
55 // the same GCInfo, have later threads reuse the slot
56 // allocated by the first.
57 if (*gcInfoIndexSlot)
58 return;
59
60 int index = ++s_gcInfoIndex;
54 size_t gcInfoIndex = static_cast<size_t>(index); 61 size_t gcInfoIndex = static_cast<size_t>(index);
55 ASSERT(gcInfoIndex < GCInfoTable::maxIndex); 62 ASSERT(gcInfoIndex < GCInfoTable::maxIndex);
56 if (gcInfoIndex >= s_gcInfoTableSize) 63 if (gcInfoIndex >= s_gcInfoTableSize)
57 resize(gcInfoIndex); 64 resize();
58 65
59 return gcInfoIndex; 66 s_gcInfoTable[gcInfoIndex] = gcInfo;
67 *gcInfoIndexSlot = gcInfoIndex;
60 } 68 }
61 69
62 void GCInfoTable::resize(size_t index) 70 void GCInfoTable::resize()
63 { 71 {
64 AtomicallyInitializedStatic(Mutex&, mutex = *new Mutex);
65 MutexLocker locker(mutex);
66
67 // Keep a lock while expanding the shared table; check
68 // if another thread have already resized.
69 if (index < s_gcInfoTableSize)
70 return;
71
72 // (Light) experimentation suggests that Blink doesn't need 72 // (Light) experimentation suggests that Blink doesn't need
73 // more than this while handling content on popular web properties. 73 // more than this while handling content on popular web properties.
74 const size_t initialSize = 512; 74 const size_t initialSize = 512;
75 75
76 size_t newSize = s_gcInfoTableSize ? 2 * s_gcInfoTableSize : initialSize; 76 size_t newSize = s_gcInfoTableSize ? 2 * s_gcInfoTableSize : initialSize;
77 ASSERT(newSize < GCInfoTable::maxIndex); 77 ASSERT(newSize < GCInfoTable::maxIndex);
78 s_gcInfoTable = reinterpret_cast<GCInfo const**>(realloc(s_gcInfoTable, newS ize * sizeof(GCInfo))); 78 s_gcInfoTable = reinterpret_cast<GCInfo const**>(realloc(s_gcInfoTable, newS ize * sizeof(GCInfo)));
79 ASSERT(s_gcInfoTable); 79 ASSERT(s_gcInfoTable);
80 memset(reinterpret_cast<uint8_t*>(s_gcInfoTable) + s_gcInfoTableSize * sizeo f(GCInfo), 0, (newSize - s_gcInfoTableSize) * sizeof(GCInfo)); 80 memset(reinterpret_cast<uint8_t*>(s_gcInfoTable) + s_gcInfoTableSize * sizeo f(GCInfo), 0, (newSize - s_gcInfoTableSize) * sizeof(GCInfo));
81 s_gcInfoTableSize = newSize; 81 s_gcInfoTableSize = newSize;
82 } 82 }
83 83
84 void GCInfoTable::init() 84 void GCInfoTable::init()
85 { 85 {
86 RELEASE_ASSERT(!s_gcInfoTable); 86 RELEASE_ASSERT(!s_gcInfoTable);
87 resize(0); 87 resize();
88 } 88 }
89 89
90 void GCInfoTable::shutdown() 90 void GCInfoTable::shutdown()
91 { 91 {
92 free(s_gcInfoTable); 92 free(s_gcInfoTable);
93 s_gcInfoTable = nullptr; 93 s_gcInfoTable = nullptr;
94 } 94 }
95 95
96 StackFrameDepth* Visitor::m_stackFrameDepth = nullptr; 96 StackFrameDepth* Visitor::m_stackFrameDepth = nullptr;
97 97
98 #if ENABLE(ASSERT) 98 #if ENABLE(ASSERT)
99 void assertObjectHasGCInfo(const void* payload, size_t gcInfoIndex) 99 void assertObjectHasGCInfo(const void* payload, size_t gcInfoIndex)
100 { 100 {
101 HeapObjectHeader::fromPayload(payload)->checkHeader(); 101 HeapObjectHeader::fromPayload(payload)->checkHeader();
102 #if !defined(COMPONENT_BUILD) 102 #if !defined(COMPONENT_BUILD)
103 // On component builds we cannot compare the gcInfos as they are statically 103 // On component builds we cannot compare the gcInfos as they are statically
104 // defined in each of the components and hence will not match. 104 // defined in each of the components and hence will not match.
105 BaseHeapPage* page = pageFromObject(payload); 105 BaseHeapPage* page = pageFromObject(payload);
106 ASSERT(page->orphaned() || HeapObjectHeader::fromPayload(payload)->gcInfoInd ex() == gcInfoIndex); 106 ASSERT(page->orphaned() || HeapObjectHeader::fromPayload(payload)->gcInfoInd ex() == gcInfoIndex);
107 #endif 107 #endif
108 } 108 }
109 #endif 109 #endif
110 110
111 } 111 }
OLDNEW
« no previous file with comments | « Source/platform/heap/Visitor.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698