[
https://issues.apache.org/jira/browse/GEODE-9078?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17344645#comment-17344645
]
ASF GitHub Bot commented on GEODE-9078:
---------------------------------------
gaussianrecurrence commented on pull request #776:
URL: https://github.com/apache/geode-native/pull/776#issuecomment-841298887
Regarding the pending topic we had related to having only read locks in
PdxType, I went through the possible cases in which PdxType state is written
and where is read I came to the following conclusion (Sorry for the long read):
## Cases in which PdxType state is accessed
- **toData**
- **(fixed|variable)LengthFieldPosition**
- **get(RemoteToLocal|LocalToRemote)Map**. This it uses lazy initialization,
that's why is considered here.
- **isContains**
- **clone**
- **is(Local|Remote)TypeContains**
- **mergeVersion**
- **operator==**
If PdxType requires exlusive access for those cases where its state is
mutated, then in all of the above cases, a shared access lock will be required.
## Cases in which PdxType state is mutated
- **fromData**. This is only used while requesting a PdxType to the server,
so no other thread will access this type. Hence, this function does not require
exclusive access.
- **add(Fixed|Variable)LengthTypeField** are only used while creating the
PdxType, so it won't require exclusive access.
- **InitializeType** is called always after a PdxType has been created or
fetched from the server, so it won't require exclusive access.
- **init(LocalToRemote|RemoteToLocal)** is called in
get(LocalToRemote|RemoteToLocal)Map and InitializeType. Given InitializeType
does not require exclusive access, whether or not
init(LocalToRemote|RemoteToLocal) requires exclusive access depends on
get(LocalToRemote|RemoteToLocal)Map.
- **get(RemoteToLocal|LocalToRemote)Map** uses lazy initialization, meaning
that if init(LocalToRemote|RemoteToLocal) was previously called, it won't
mutate the state, and it's used by:
- **PdxRemoteWriter::PdxRemoteWriter**
- **PdxRemoteWriter::writeUnreadFields**
- **PdxLocalReader::PdxLocalReader**
- **PdxLocalReader::getPreservedData**
- **PdxRemoteReader::PdxRemoteReader** (by inheritance)
- **PdxReaderWithTypeCollector::PdxReaderWithTypeCollector** (by
inheritance)
### **get(RemoteToLocal|LocalToRemote)Map** follow up
So, regarding **PdxRemoteWriter**, the PdxType is either initialized to:
- A merged PdxType which is initialized in all cases.
- A local PdxType which is initialized in all cases
Regarding **PdxLocalReader**, **PdxRemoteReader**,
**PdxReaderWithTypeCollector**, it uses a PdxType instance which might be
fetched from the PdxTypeRegistry, so the exclusive access depends on whether
all the PdxTypes added to the PdxTypeRegistry are initialized.
All the calls to PdxTypeRegistry::addPdxType are in PdxHelper and in all
cases PdxTypes are initialized.
Consequently, I can conclude that **get(RemoteToLocal|LocalToRemote)Map**
does not require exlusive access.
With all of the above, I'd say there is no need to use locks in PdxTypes, so
I will remove the mutex and shared lock from that class.
I guess probably those locks were left in the code by mistake after some
refactor. I sadly could not trace the exact commit, as the change seems to have
been done before open sourcing the native client.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Remove ACE mutexes
> ------------------
>
> Key: GEODE-9078
> URL: https://issues.apache.org/jira/browse/GEODE-9078
> Project: Geode
> Issue Type: Task
> Components: native client
> Reporter: Mario Salazar de Torres
> Assignee: Mario Salazar de Torres
> Priority: Major
> Labels: obliterate-ace, pull-request-available
>
> *AS AN* geode-native contributor
> *I WANT TO* remove all occurrences of ACE mutexes
> *SO THAT* we can get rid of ACE for good
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
