[SERVER-26126] LockState caching in OperationContextImpl is broken Created: 15/Sep/16 Updated: 05/Apr/17 Resolved: 02/Feb/17 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | Internal Code |
| Affects Version/s: | None |
| Fix Version/s: | 3.5.3 |
| Type: | Improvement | Priority: | Major - P3 |
| Reporter: | Mathias Stearn | Assignee: | Geert Bosch |
| Resolution: | Done | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||
| Backwards Compatibility: | Fully Compatible | ||||
| Sprint: | Storage 2016-12-12, Storage 2017-01-23, Storage 2017-02-13 | ||||
| Participants: | |||||
| Linked BF Score: | 0 | ||||
| Description |
|
In the OperationContextImpl destructor we create a new locker then immediately destroy it. If the point of caching is that construction/destruction of LockState is too expensive, then we are are still paying the cost and getting nothing out of it. We should consider removing the cache rather than fixing it. |
| Comments |
| Comment by Githook User [ 02/Feb/17 ] |
|
Author: {u'username': u'GeertBosch', u'name': u'Geert Bosch', u'email': u'geert@mongodb.com'}Message: |
| Comment by Githook User [ 27/Dec/16 ] |
|
Author: {u'username': u'GeertBosch', u'name': u'Geert Bosch', u'email': u'geert@mongodb.com'}Message: Revert " |
| Comment by Githook User [ 23/Dec/16 ] |
|
Author: {u'username': u'GeertBosch', u'name': u'Geert Bosch', u'email': u'geert@mongodb.com'}Message: |
| Comment by Githook User [ 22/Dec/16 ] |
|
Author: {u'username': u'GeertBosch', u'name': u'Geert Bosch', u'email': u'geert@mongodb.com'}Message: |
| Comment by Esha Maharishi (Inactive) [ 13/Dec/16 ] |
|
Whoops, thanks for the reminder ian.whalen. |
| Comment by Ian Whalen (Inactive) [ 13/Dec/16 ] |
|
esha.maharishi thanks for be pro-active about reverting. small nit: please make sure to re-open an issue when doing so or it's possible the original developer might not know to return to it. |
| Comment by Githook User [ 13/Dec/16 ] |
|
Author: {u'username': u'EshaMaharishi', u'name': u'Esha Maharishi', u'email': u'esha.maharishi@mongodb.com'}Message: Revert " This reverts commit 6dc9fc6ba93b62830dd905f6fac39e0802566a9a. |
| Comment by Githook User [ 12/Dec/16 ] |
|
Author: {u'username': u'GeertBosch', u'name': u'Geert Bosch', u'email': u'geert@mongodb.com'}Message: |
| Comment by Geert Bosch [ 08/Dec/16 ] |
|
I can confirm that the cache was added to address condvar construction/destruction performance on Windows. This should show up in the micro benchmarks. |
| Comment by Ian Whalen (Inactive) [ 19/Sep/16 ] |
|
at the very least we should try to do some manual perf testing on windows before 3.4 (since there are currently no windows boxes up and running our automated tests). |
| Comment by Andy Schwerin [ 15/Sep/16 ] |
|
Is it broken on 3.2? I may have broken it on master. However, if we aren't seeing perf regression on master from it, I'd be thrilled to delete the cache. That said, perf coverage on Windows is limited, and the purpose of caching may have been to amortize the cost of Windows condvar construction. |