-
Notifications
You must be signed in to change notification settings - Fork 142
[MRESOLVER-132] Remove synchronization in TrackingFileManager #67
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/TrackingFileManager.java
Show resolved
Hide resolved
maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/TrackingFileManager.java
Show resolved
Hide resolved
|
@elharo I'd gently reuse my statement here for the Manager: #66 (comment). We can create a separate issue to improve the code. |
|
@dantran Could you also kindly try this one for you. In my opinion this removes unnecessary locks. Use the same project as for the Redisson solution, but w/o Redisson. |
|
@michael-o I build MRESOLVER-132 branch together with maven 3.6.3 tag from the source. Still see the slow build. |
|
@michael-o Did you leave out any additional |
|
@michael-o, mine is a clean build only involves this branch ( ie no SyncContextFactory) It is 10 minutes faster when using pure maven-resolver-1.5.1-SNAPSHOT at master |
|
@dantran Thanks, if is supposed to be faster. This unnecessary sync is gone. |
eolivelli
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
I left one little suggestion for an improvement, it could be significant on slow FS and probably more on Windows
Btw the change is good even in this form
maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/TrackingFileManager.java
Show resolved
Hide resolved
eolivelli
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense.
+1
|
Resolve #896 |
1 similar comment
|
Resolve #896 |
See motivation in the ticket as well as the call hierarchy: https://issues.apache.org/jira/browse/MRESOLVER-132