[
https://issues.apache.org/jira/browse/MRESOLVER-132?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Michael Osipov updated MRESOLVER-132:
-------------------------------------
Description:
Motivation: The {{TrackingFileManager}} is package private, thus never used
externally. It uses the interned string reperesentation of the canonical path
and file locking. We all know that file locking in a single JVM are pointless,
it will lead to race conditions. This is why the locking is multistage. Single
JVM and file locks for multi JVM. The interesting part is that if you look up
the call stack of {{TrackingFileManager#read(File)}} and
{{TrackingFileManager#update(File, Map<String, String>)}} you'll see that all
of them are wrapped in a {{SyncContext}} which already adds locks for them. I
don't understand why this is done here. If outer lock works, I see no need to
lock again somewhere deeper. I am also keen to say that this kind of
synchronization can be completely removed if you have a decent
{{SyncContextFactory}} implementation.
The {{TrackingFileManager}} has been overridden with MRESOLVER-130 and
MRESOLVER-131 for obvious reasons.
In short: the locking (synchronization) has to happen in a sync context, not
here. Especially because the canonicalization of paths adds an unnecessary
performance penalty.
was:
Motivation: The {{TrackingFileManager}} is package private, thus never used
externally. It uses the interned string reperesentation of the canonical path
and file locking. We all know that file locking in a single JVM are pointless,
it will lead to race conditions. This is why the locking is multistage. Single
JVM and file locks for multi JVM. The interesting part is that if you look up
the call stack of {{TrackingFileManager#read(File)}} and
{{TrackingFileManager#update(File, Map<String, String>)}} you'll see that all
of them are wrapped in a {{SyncContext}} which already adds locks for them. I
don't understand why this is done here. If outer lock works, I see no need to
lock again somewhere deeper. I am also keen to say that this kind of
synchronization can be completely removed if you have a decent
{{SyncContextFactory}} implementation.
I am inclined to override the {{TrackingFileManager}} with MRESOLVER-130 and
MRESOLVER-131 for obvious reasons.
In short: the locking (synchronization) has to happen in a sync context, not
here. Especially because the canonicalization of paths adds an unnecessary
performance penalty.
> Remove synchronization in TrackingFileManager
> ---------------------------------------------
>
> Key: MRESOLVER-132
> URL: https://issues.apache.org/jira/browse/MRESOLVER-132
> Project: Maven Resolver
> Issue Type: Task
> Components: resolver
> Affects Versions: 1.5.0
> Reporter: Michael Osipov
> Assignee: Michael Osipov
> Priority: Major
> Attachments: call-hierarchy-read.png, call-hierarchy-write.png
>
>
> Motivation: The {{TrackingFileManager}} is package private, thus never used
> externally. It uses the interned string reperesentation of the canonical path
> and file locking. We all know that file locking in a single JVM are
> pointless, it will lead to race conditions. This is why the locking is
> multistage. Single JVM and file locks for multi JVM. The interesting part is
> that if you look up the call stack of {{TrackingFileManager#read(File)}} and
> {{TrackingFileManager#update(File, Map<String, String>)}} you'll see that all
> of them are wrapped in a {{SyncContext}} which already adds locks for them. I
> don't understand why this is done here. If outer lock works, I see no need to
> lock again somewhere deeper. I am also keen to say that this kind of
> synchronization can be completely removed if you have a decent
> {{SyncContextFactory}} implementation.
> The {{TrackingFileManager}} has been overridden with MRESOLVER-130 and
> MRESOLVER-131 for obvious reasons.
> In short: the locking (synchronization) has to happen in a sync context, not
> here. Especially because the canonicalization of paths adds an unnecessary
> performance penalty.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
