Modify

Ticket #2361 (closed Feature Requests: fixed)

Opened 6 years ago

Last modified 20 months ago

thread_specific_ptr: nature of the key, complexity and rationale

Reported by: vicente.botet@… Owned by: viboes
Milestone: Boost 1.52.0 Component: thread
Version: Boost 1.36.0 Severity: Not Applicable
Keywords: thread_specific_ptr Cc:

Description

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

Attachments

tss.patch Download (38.4 KB) - added by andysem 5 years ago.
The patch restores constant-time complexity of thread_specific_ptr

Change History

Changed 5 years ago by andysem

The patch restores constant-time complexity of thread_specific_ptr

comment:1 follow-up: ↓ 4 Changed 5 years ago by 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.

comment:2 Changed 5 years ago by andysem

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.

comment:3 Changed 4 years ago by 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?

comment:4 in reply to: ↑ 1 Changed 2 years ago by 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

comment:5 Changed 2 years ago by andysem

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.

comment:6 Changed 2 years ago by viboes

  • Owner changed from anthonyw to viboes
  • Status changed from new to assigned
  • Milestone changed from Boost 1.37.0 to To Be Determined

comment:7 Changed 20 months ago by viboes

  • Milestone changed from To Be Determined to 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.

comment:8 Changed 20 months ago by viboes

Committed in trunk at revision [80198].

comment:9 follow-up: ↓ 10 Changed 20 months ago by anonymous

That should be Rationale, not Rational, right?

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

comment:10 in reply to: ↑ 9 Changed 20 months ago by 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.

comment:11 Changed 20 months ago by viboes

  • Status changed from assigned to closed
  • Resolution set to fixed

Committed revision 80516.

View

Add a comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
The resolution will be deleted. Next status will be 'reopened'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.