--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/415f44d6-88ed-476c-a6e1-cacccfe2cc85%40chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAJTZ7LJTH9H54OikYw5L9beNoVfXxpLO9BcRw17_5MJLiz5yrA%40mail.gmail.com.
--
I don't think we would want to use anything but the base implementation of TLS.On Android, there are very few TLS slots (I found a reference to 60 somewhere) and were hitting the maximum. The base implementation maps a 256-entry TLS array into a single OS TLS slot to avoid this problem. I would assume that an STL implementation of TLS would just call the native OS APIs and if people start using them we would be running out of slots again.We should probably put this on the explicitly banned list.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CA%2BapAgH4NoB5wjE_P5P6GJ2ZrB3E3fRQXUw%2BA-u%3D1wsyyafuPw%40mail.gmail.com.
I don't think we would want to use anything but the base implementation of TLS.On Android, there are very few TLS slots (I found a reference to 60 somewhere) and were hitting the maximum. The base implementation maps a 256-entry TLS array into a single OS TLS slot to avoid this problem. I would assume that an STL implementation of TLS would just call the native OS APIs and if people start using them we would be running out of slots again.
We should probably put this on the explicitly banned list.
Brett--On Mon, Aug 14, 2017 at 10:11 AM, Ken Rockot <roc...@chromium.org> wrote:--I see there is currently no usage of this in the tree, and I would like to introduce some. To my surprise I can find no evidence of prior discussion.What's the deal? Can we use it? If not, is there a suitable alternative?Specifically of interest is a need to ensure that the lifetime of my thread-local thing is actually bounded by each thread's lifetime, and our //base thread-local support does not appear to be sufficient for such things.
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/415f44d6-88ed-476c-a6e1-cacccfe2cc85%40chromium.org.
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CABiGVV8t8SG3kRp8d1jh9YT5stzcmfEbv%2Bku-0Nb2aJpJSTZBg%40mail.gmail.com.
On Tue, Aug 15, 2017 at 3:04 PM, Brett Wilson <bre...@chromium.org> wrote:I don't think we would want to use anything but the base implementation of TLS.On Android, there are very few TLS slots (I found a reference to 60 somewhere) and were hitting the maximum. The base implementation maps a 256-entry TLS array into a single OS TLS slot to avoid this problem. I would assume that an STL implementation of TLS would just call the native OS APIs and if people start using them we would be running out of slots again.There may be some valid concerns about thread_local but this should not be one of them (it is not technically feasible to map thread_local variables to individual TLS slots provided by the host OS).We should probably put this on the explicitly banned list.What's the rule of thumb here? I'm new to Chrome but I'd expect that all thing being equal we'd favor standard language features over hand crafted alternatives. Thread storage class is a first class, fundamental part of C++ since C++11, with well defined semantics (and one would expect first class, solid implementations in all the major toolchains)
MSVC places thread local variables(*) in a special section (.tls) . A single TLS slot points to the base of the thread-specific copy of this section so accessing a thread_local variable is .tls_base[offset]. I'll let Linux/macOS experts chime in about how things are done on these platforms although it seems very close to the PE/COFF solution.(*) MSVC implements thread_local pretty much by repackaging of the old declspec(thread) with a few rough corners (around initialization) fixed.
On Tue, Aug 15, 2017 at 3:56 PM, Brett Wilson <bre...@chromium.org> wrote:On Tue, Aug 15, 2017 at 3:47 PM, Leonard Mosescu <mos...@chromium.org> wrote:On Tue, Aug 15, 2017 at 3:04 PM, Brett Wilson <bre...@chromium.org> wrote:I don't think we would want to use anything but the base implementation of TLS.On Android, there are very few TLS slots (I found a reference to 60 somewhere) and were hitting the maximum. The base implementation maps a 256-entry TLS array into a single OS TLS slot to avoid this problem. I would assume that an STL implementation of TLS would just call the native OS APIs and if people start using them we would be running out of slots again.There may be some valid concerns about thread_local but this should not be one of them (it is not technically feasible to map thread_local variables to individual TLS slots provided by the host OS).We should probably put this on the explicitly banned list.What's the rule of thumb here? I'm new to Chrome but I'd expect that all thing being equal we'd favor standard language features over hand crafted alternatives. Thread storage class is a first class, fundamental part of C++ since C++11, with well defined semantics (and one would expect first class, solid implementations in all the major toolchains)If the implementation is good and does what we want on all platforms for all compilers we support and there are no specific concerns, then we should use it. I am not so optimistic as you about uniformly good implementations though...Brett
--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CANFFm8g_hFQg_M1afPyJ2uWAkBi1cjAdqzHwvykEPF-hKFq27A%40mail.gmail.com.
Also worth mentioning in this thread: base now provides SequenceLocalStorageSlot as a sequence-friendly version of ThreadLocalStorage::Slot. As we move more and more towards sequences, I suspect most TLS usage outside of base internals should migrate to SLS.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CANFFm8h8BR04WUzsKX8MY_tu1xV%3Do0QrqPMhJemzXS-NsyMYQA%40mail.gmail.com.
The caveat on Mac is that we cannot access TLS objects during the destructors of other TLS objects, which seems fairly reasonable
The caveat on Mac is that we cannot access TLS objects during the destructors of other TLS objects, which seems fairly reasonableIt's worth noting that this is by design and not an implementation issue. It's pretty much the same constraint that applies to global objects.
What I found is that on Mac, having a TLS object will automatically generate and accessor function, which dynamically allocates memory for the object on first access on that thread. They get destroyed/deallocated on thread destruction, which means we shouldn't access even POD TLS data on the destructors.I still think this is a reasonable limitation given the benefits of improved performance and better generated code.As for the specific issue you mentioned with AtomicStrings, I haven't seen that happening, it's possible that it got fixed. In particular, this would also be bad even without thread_local, since accessing an AtomicString after the destruction on the WTFThreadData would lead to leaked memory, since the WTFThreadData would be re-allocated and nothing will free it afterwards. I'm fine if that gets changed to a crash, since it should be fixed anyway.
static void TLSThreadTest();
static void TLSThreadTest2();
class TLSHelperB {
public:
TLSHelperB() {
LOG(ERROR) << this << " " << __PRETTY_FUNCTION__;
}
~TLSHelperB() {
LOG(ERROR) << this << " " << __PRETTY_FUNCTION__;
TLSThreadTest();
}
};
class WTFTLSHelperB {
public:
WTFTLSHelperB() {
LOG(ERROR) << this << " " << __PRETTY_FUNCTION__;
}
~WTFTLSHelperB() {
LOG(ERROR) << this << " " << __PRETTY_FUNCTION__;
TLSThreadTest2();
}
};
class TLSHelperA {
public:
TLSHelperA() {
static thread_local TLSHelperB tls_helper;
DEFINE_THREAD_SAFE_STATIC_LOCAL(WTF::ThreadSpecific<WTFTLSHelperB>, tls_helper_wtf, ());
*tls_helper_wtf;
LOG(ERROR) << this << " " << __PRETTY_FUNCTION__;
}
~TLSHelperA() {
LOG(ERROR) << this << " " << __PRETTY_FUNCTION__;
}
};
class WTFTLSHelperA {
public:
WTFTLSHelperA() {
static thread_local TLSHelperB tls_helper;
DEFINE_THREAD_SAFE_STATIC_LOCAL(WTF::ThreadSpecific<WTFTLSHelperB>, tls_helper_wtf, ());
*tls_helper_wtf;
LOG(ERROR) << this << " " << __PRETTY_FUNCTION__;
}
~WTFTLSHelperA() {
LOG(ERROR) << this << " " << __PRETTY_FUNCTION__;
}
};
static void TLSThreadTest() {
LOG(ERROR) << "Start " << __PRETTY_FUNCTION__;
static thread_local TLSHelperA tls_helper_a;
DEFINE_THREAD_SAFE_STATIC_LOCAL(WTF::ThreadSpecific<WTFTLSHelperA>, tls_helper_wtf, ());
*tls_helper_wtf;
LOG(ERROR) << "End " << __PRETTY_FUNCTION__;
}
static void TLSThreadTest2() {
LOG(ERROR) << "Start " << __PRETTY_FUNCTION__;
static thread_local TLSHelperA tls_helper_a;
DEFINE_THREAD_SAFE_STATIC_LOCAL(WTF::ThreadSpecific<WTFTLSHelperA>, tls_helper_wtf, ());
*tls_helper_wtf;
LOG(ERROR) << "End " << __PRETTY_FUNCTION__;
}
TEST(..., TLS) {
Thread a("Test");
EXPECT_TRUE(a.Start());
a.task_runner()->PostTask(
FROM_HERE,
base::BindOnce(&TLSThreadTest));
a.task_runner()->PostTask(
FROM_HERE,
base::BindOnce(&TLSThreadTest2));
a.Stop();
}
And here's the output:
Start void WTF::TLSThreadTest()
0x7ffef6e08582 WTF::TLSHelperB::TLSHelperB()
0x27fae18038 WTF::WTFTLSHelperB::WTFTLSHelperB()
0x7ffef6e08580 WTF::TLSHelperA::TLSHelperA()
0x7ffef6e08590 WTF::TLSHelperB::TLSHelperB()
0x27fae18088 WTF::WTFTLSHelperB::WTFTLSHelperB()
0x27fae18060 WTF::WTFTLSHelperA::WTFTLSHelperA()
End void WTF::TLSThreadTest()
Start void WTF::TLSThreadTest2()
0x7ffef6e085a0 WTF::TLSHelperA::TLSHelperA()
0x27fae180b0 WTF::WTFTLSHelperA::WTFTLSHelperA()
End void WTF::TLSThreadTest2()
0x7ffef6e085a0 WTF::TLSHelperA::~TLSHelperA()
0x7ffef6e08590 WTF::TLSHelperB::~TLSHelperB()
Start void WTF::TLSThreadTest()
End void WTF::TLSThreadTest()
0x7ffef6e08580 WTF::TLSHelperA::~TLSHelperA()
0x7ffef6e08582 WTF::TLSHelperB::~TLSHelperB()
Start void WTF::TLSThreadTest()
End void WTF::TLSThreadTest()
0x27fae18038 WTF::WTFTLSHelperB::~WTFTLSHelperB()
Start void WTF::TLSThreadTest2()
0x7ffef6e07d62 WTF::TLSHelperB::TLSHelperB()
0x7ffef6e07d80 WTF::TLSHelperA::TLSHelperA()
End void WTF::TLSThreadTest2()
0x27fae18060 WTF::WTFTLSHelperA::~WTFTLSHelperA()
0x27fae18088 WTF::WTFTLSHelperB::~WTFTLSHelperB()
Start void WTF::TLSThreadTest2()
End void WTF::TLSThreadTest2()
0x27fae180b0 WTF::WTFTLSHelperA::~WTFTLSHelperA()
0x7ffef6e07d80 WTF::TLSHelperA::~TLSHelperA()
0x7ffef6e07d62 WTF::TLSHelperB::~TLSHelperB()
Start void WTF::TLSThreadTest()
0x27fae180b0 WTF::WTFTLSHelperB::WTFTLSHelperB()
0x7ffef6e07d60 WTF::TLSHelperA::TLSHelperA()
0x7ffef6e07d70 WTF::TLSHelperB::TLSHelperB()
0x27fae18060 WTF::WTFTLSHelperB::WTFTLSHelperB()
0x27fae18088 WTF::WTFTLSHelperA::WTFTLSHelperA()
End void WTF::TLSThreadTest()
0x27fae180b0 WTF::WTFTLSHelperB::~WTFTLSHelperB()
Start void WTF::TLSThreadTest2()
0x7ffef6e07d62 WTF::TLSHelperB::TLSHelperB()
0x7ffef6e07d80 WTF::TLSHelperA::TLSHelperA()
0x7ffef6e07d70 WTF::TLSHelperB::TLSHelperB()
0x27fae18038 WTF::WTFTLSHelperA::WTFTLSHelperA()
End void WTF::TLSThreadTest2()
0x27fae18088 WTF::WTFTLSHelperA::~WTFTLSHelperA()
0x27fae18060 WTF::WTFTLSHelperB::~WTFTLSHelperB()
Start void WTF::TLSThreadTest2()
End void WTF::TLSThreadTest2()
0x27fae18038 WTF::WTFTLSHelperA::~WTFTLSHelperA()
0x7ffef6e07d70 WTF::TLSHelperB::~TLSHelperB()
Start void WTF::TLSThreadTest()
0x27fae18038 WTF::WTFTLSHelperB::WTFTLSHelperB()
0x7ffef6e07d60 WTF::TLSHelperA::TLSHelperA()
0x27fae18088 WTF::WTFTLSHelperB::WTFTLSHelperB()
0x27fae18060 WTF::WTFTLSHelperA::WTFTLSHelperA()
End void WTF::TLSThreadTest()
0x7ffef6e07d80 WTF::TLSHelperA::~TLSHelperA()
0x7ffef6e07d62 WTF::TLSHelperB::~TLSHelperB()
Start void WTF::TLSThreadTest()
End void WTF::TLSThreadTest()
0x7ffef6e07d70 WTF::TLSHelperB::~TLSHelperB()
Start void WTF::TLSThreadTest()
End void WTF::TLSThreadTest()
0x7ffef6e07d60 WTF::TLSHelperA::~TLSHelperA()
0x27fae18038 WTF::WTFTLSHelperB::~WTFTLSHelperB()
Start void WTF::TLSThreadTest2()
0x7ffef6e07d62 WTF::TLSHelperB::TLSHelperB()
0x7ffef6e07d80 WTF::TLSHelperA::TLSHelperA()
0x7ffef6e07d70 WTF::TLSHelperB::TLSHelperB()
0x27fae180b0 WTF::WTFTLSHelperA::WTFTLSHelperA()
End void WTF::TLSThreadTest2()
0x27fae18060 WTF::WTFTLSHelperA::~WTFTLSHelperA()
0x27fae18088 WTF::WTFTLSHelperB::~WTFTLSHelperB()
Start void WTF::TLSThreadTest2()
End void WTF::TLSThreadTest2()
0x27fae180b0 WTF::WTFTLSHelperA::~WTFTLSHelperA()
0x7ffef6e07d70 WTF::TLSHelperB::~TLSHelperB()
Start void WTF::TLSThreadTest()
0x27fae180b0 WTF::WTFTLSHelperB::WTFTLSHelperB()
0x7ffef6e07d60 WTF::TLSHelperA::TLSHelperA()
0x27fae18060 WTF::WTFTLSHelperB::WTFTLSHelperB()
0x27fae18088 WTF::WTFTLSHelperA::WTFTLSHelperA()
End void WTF::TLSThreadTest()
0x7ffef6e07d80 WTF::TLSHelperA::~TLSHelperA()
0x7ffef6e07d62 WTF::TLSHelperB::~TLSHelperB()
Start void WTF::TLSThreadTest()
End void WTF::TLSThreadTest()
0x7ffef6e07d60 WTF::TLSHelperA::~TLSHelperA()
0x27fae180b0 WTF::WTFTLSHelperB::~WTFTLSHelperB()
Start void WTF::TLSThreadTest2()
0x7ffef6e07e82 WTF::TLSHelperB::TLSHelperB()
0x7ffef6e07ea0 WTF::TLSHelperA::TLSHelperA()
0x7ffef6e07e90 WTF::TLSHelperB::TLSHelperB()
0x27fae18038 WTF::WTFTLSHelperA::WTFTLSHelperA()
End void WTF::TLSThreadTest2()
0x27fae18088 WTF::WTFTLSHelperA::~WTFTLSHelperA()
0x27fae18060 WTF::WTFTLSHelperB::~WTFTLSHelperB()
Start void WTF::TLSThreadTest2()
End void WTF::TLSThreadTest2()
0x27fae18038 WTF::WTFTLSHelperA::~WTFTLSHelperA()
You received this message because you are subscribed to the Google Groups "cxx" group.
To unsubscribe from this group and stop receiving emails from it, send an email to cxx+uns...@chromium.org.
To post to this group, send email to c...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CACuR13chS9Q_xUCVJ%3D0jkq4sLbVsBYUqni50sY4E-GWNj4%3D-JA%40mail.gmail.com.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CALQVofpGyD4LdqkaQE9Fb-8v3Rm2BBN4n2NYfr_4j7o7SJ8sLw%40mail.gmail.com.
OK. I'm a little frustrated that the C++ spec doesn't seem to explicitly spell out what happens in cases like this, but it sounds like this is safe at least for a simple POD like is_main_thread, and we can see what issues we run into in practice from there. If thakis or danakj agrees, I'm happy to approve a CL which moves it to allowed with use by special dispensation and adds a use for a simple such case. I'm reluctant to give carte-blanche for broader use until we have more experience with how this interacts with the various other TLS in Chromium (because it tickles so many weird platform/compiler-specific things), but I would like to see us ultimately use the language-level thread_local if we can, because it's pretty nice. :)
thread_local
, but thread_local
with non-trivial destructors requires run time support that wasn't available until M (and I think the NDK's current libc++abi doesn't have the support either). There was actually a patch recently made to libc++abi adding support for this on platforms that don't have native support, so once that lands and we pick it up we could actually support that on older platforms."""To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev+unsubscribe@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CACuR13d0FnS0Nga7EEEu%2BwdWciB1%2BRrUjzYQXTGvYdK88GZkSg%40mail.gmail.com.
On Tue, Nov 28, 2017 at 11:08 PM, Jeremy Roman <jbr...@chromium.org> wrote:OK. I'm a little frustrated that the C++ spec doesn't seem to explicitly spell out what happens in cases like this, but it sounds like this is safe at least for a simple POD like is_main_thread, and we can see what issues we run into in practice from there. If thakis or danakj agrees, I'm happy to approve a CL which moves it to allowed with use by special dispensation and adds a use for a simple such case. I'm reluctant to give carte-blanche for broader use until we have more experience with how this interacts with the various other TLS in Chromium (because it tickles so many weird platform/compiler-specific things), but I would like to see us ultimately use the language-level thread_local if we can, because it's pretty nice. :)For the record, I beliieve that thread_local was broken Android before Android M :-( [1]. It is possible to work around this by using a more recent version of the Android NDK and /or libc++abi sources though.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CACnJMqqnp6nT-1iiV7oBHNXGO%2BkL6L2B3drGHjVHjXgqK43upQ%40mail.gmail.com.
We're still on r12b. A Skia change went in recently that had a fix we needed for Chromium to compile with r16 (that's probably what you saw). The Chromium-side changes seem to all work, so barring any unexpected blockers, r16 will probably be in shortly after branch point (I would guess early next week?).
thakis/danakj, want to comment on the above? It seems like this thread is a bit in limbo right now, I'd like to see it move to a resolved place.
PK
--
You received this message because you are subscribed to the Google Groups "cxx" group.
To unsubscribe from this group and stop receiving emails from it, send an email to cxx+uns...@chromium.org.
To post to this group, send email to c...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CAAHOzFDK9Y5OuzoG%3DcnGeN%2BC5kYAj3CEms7SWGvPY%2BYuCx7TMA%40mail.gmail.com.
--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/54654263-36c5-46a9-9d73-788185208794%40chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CALz_-TYPChvLOzPkHfG947dO7vkZ6Km-nnbSwnvcRThTKwhYnA%40mail.gmail.com.
Has there been any updates on the experiments to use thread_local in Chromium? We're interested in using it in a third-party dependency of Chromium (Dawn), maybe that can be the experiment.Cheers,Corentin
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CACuR13cbtMOmOGFe0pg%2B-vOLRHnjhr2FqiJ-PDTN6_B87J9JVQ%40mail.gmail.com.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev+unsubscribe@chromium.org.