thread_specific_ptr: nature of the key, complexity and rationale

[vicente.botet@…]

I didn't know that the key of thread_specific_ptr was the address of the variable. I expected a constant complexity for boost::thread_specific_ptr, but if the key is the address this seams not possible. I supose you had some good raison to not use an index key instead of the address.

Please, could you add the nature of the key, the complexity of the boost::thread_specific_ptr and the rationale on the design choice on the documentation.

Ticket requested by A. W. on ​http://www.nabble.com/-thread--TSS-cleanup-to19590361.html#a19623640

[Andrey Semashev]

The patch restores constant-time complexity of thread_specific_ptr

[Andrey Semashev]

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]

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]

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]

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]

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.

[viboes]

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]

Committed in trunk at revision [80198].

[anonymous]

That should be Rationale, not Rational, right?

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

[viboes]

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.

[viboes]

Committed revision 80516.