[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:
Depends
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: SERVER-26126 Remove broken lockState caching
Branch: master
https://github.com/mongodb/mongo/commit/6c0ede8c7518d9bb8b5d8ed0897fe26e2b0a94ad

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 "SERVER-26126 Remove broken LockState caching"
Branch: master
https://github.com/mongodb/mongo/commit/c4e1e4bbcc1a6fc1f9ca4e3cbf524e6c9c99ce1a

Comment by Githook User [ 23/Dec/16 ]

Author:

{u'username': u'GeertBosch', u'name': u'Geert Bosch', u'email': u'geert@mongodb.com'}

Message: SERVER-26126 Fix repl unittests
Branch: master
https://github.com/mongodb/mongo/commit/46daec6a95628d336762c5f78430f54d361e63c0

Comment by Githook User [ 22/Dec/16 ]

Author:

{u'username': u'GeertBosch', u'name': u'Geert Bosch', u'email': u'geert@mongodb.com'}

Message: SERVER-26126 Remove broken LockState caching
Branch: master
https://github.com/mongodb/mongo/commit/7b68733fae7ce8d91f9a5fda67b9972c15a20ef0

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 "SERVER-26126 Remove broken LockState caching"

This reverts commit 6dc9fc6ba93b62830dd905f6fac39e0802566a9a.
Branch: master
https://github.com/mongodb/mongo/commit/4061adae4c4cc556954ae50924f4aadd9f8ac732

Comment by Githook User [ 12/Dec/16 ]

Author:

{u'username': u'GeertBosch', u'name': u'Geert Bosch', u'email': u'geert@mongodb.com'}

Message: SERVER-26126 Remove broken LockState caching
Branch: master
https://github.com/mongodb/mongo/commit/6dc9fc6ba93b62830dd905f6fac39e0802566a9a

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.

Generated at Thu Feb 08 04:11:13 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.