Boost C++ Libraries: Ticket #2361: thread_specific_ptr: nature of the key, complexity and rationale

attachment set

Andrey Semashev — Fri, 18 Sep 2009 20:30:11 GMT

attachment → tss.patch

The patch restores constant-time complexity of thread_specific_ptr

Andrey Semashev — Fri, 18 Sep 2009 20:41:09 GMT

FWIW, I attached the patch against branches/release that restores constant-time complexity of thread_specific_ptr, instead of linear on the number of pointers. This essentially restores the behavior that was there in previous Boost versions (1.33, for sure). The patch also fixes a couple of other issues: calling cleanup function on NULL pointers and releasing cleanup function on pointer destruction.

I ran tests on Linux (GCC 4.3) and Windows (MSVC 9) with this patch and it looked fine.

Anthony, I know you're working in a different direction in trunk, thus I realise that this patch is unlikely to be applied to the mainline. However, I'd like to encourage you to provide constant-time complexity of TSS, like it was before Boost.Thread rewrite.

Andrey Semashev — Fri, 18 Sep 2009 20:50:39 GMT

2 Vicente: I'm not in position to give official response to your questions, but I suspect that the approach Anthony took could simplify solving problems with dynamically loadable/unloadable modules (nonetheless, the current branches/release version does not behave correctly with respect to this).

For the record, my patch does suffer from this problem, too. However, I believe it is possible to provide both constant-time complexity and proper modules support. If not, well... I'd opt for performance. Perhaps, a library option should be provided to make everyone happy.

viboes — Fri, 20 Nov 2009 12:25:39 GMT

Anthony, I see that you have modified the thread_specific_ptr implementation, but I don't see yet any explanation about the complexity and the rational in the documentation> Please could you respond to this ticket?

viboes — Mon, 05 Dec 2011 18:55:04 GMT

Replying to andysem:

FWIW, I attached the patch against branches/release that restores constant-time complexity of thread_specific_ptr, instead of linear on the number of pointers. This essentially restores the behavior that was there in previous Boost versions (1.33, for sure). The patch also fixes a couple of other issues: calling cleanup function on NULL pointers and releasing cleanup function on pointer destruction.

I ran tests on Linux (GCC 4.3) and Windows (MSVC 9) with this patch and it looked fine.

Anthony, I know you're working in a different direction in trunk, thus I realise that this patch is unlikely to be applied to the mainline. However, I'd like to encourage you to provide constant-time complexity of TSS, like it was before Boost.Thread rewrite.

Andrey,

I don't know if you maintain this patch on your local system in synchronization with trunk. If this is the case could you attach it. I would like to play with to consider if your approach can be modified to take in account also modules support.

Thanks

Andrey Semashev — Mon, 05 Dec 2011 19:17:58 GMT

I'm sorry, I don't maintain the patch. I prepared it once for the Boost release I'd been working with at the time. I didn't look at the current code of thread_specific_ptr but I think it shouldn't be too hard to adapt it to the current code base. You can contact me (either in this ticket or in any other way) if you need any help on this patch.

owner, status, milestone changed

viboes — Sat, 07 Jan 2012 23:41:05 GMT

owner changed from Anthony Williams to viboes
status new → assigned
milestone Boost 1.37.0 → To Be Determined

milestone changed

viboes — Sat, 25 Aug 2012 12:24:50 GMT

milestone To Be Determined → Boost 1.52.0

Next follows the text that will be added

[heading Rational about the nature of the key]
Boost.Thread uses the address of the `thread_specific_ptr` instance as key of the thread specific pointers. This avoids to create/destroy a key which will need a lock to protect from race conditions. This has a little performance liability, as the access must be done using an associative container.

viboes — Sat, 25 Aug 2012 12:29:33 GMT

Committed in trunk at revision [80198].

anonymous — Sat, 25 Aug 2012 12:42:07 GMT

That should be Rationale, not Rational, right?

Also, are there plans to change the implementation to avoid associative container lookup?

viboes — Sun, 26 Aug 2012 15:32:04 GMT

Replying to anonymous:

That should be Rationale, not Rational, right?

I will fix it.

Also, are there plans to change the implementation to avoid associative container lookup?

Not for the next releases. Changing to a random access container needs much more reflexion. I will see what can be done in a middle future.

status changed; resolution set

viboes — Thu, 13 Sep 2012 19:11:29 GMT

status assigned → closed
resolution → fixed

Committed revision 80516.