You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@maven.apache.org by Ivan Babiankou <iv...@gmail.com> on 2021/10/16 15:21:01 UTC
Another take on [MRESOLVER-7] Download dependency POMs in parallel
I would love to prepare decent PR
<https://issues.apache.org/jira/browse/MRESOLVER-7?focusedCommentId=17171271&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17171271>
for the MRESOLVER-7 issue, so that it makes it to one of the upcoming
releases. But I could definitely use some guidance, given the history of
the ticket
<https://issues.apache.org/jira/browse/MRESOLVER-7?focusedCommentId=16885618&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-16885618>
and the fact that it’s my first contribution to Maven and OSS in general.
Is there anyone willing to point me in the right direction (in email or a
quick 15-30min call)? I would really appreciate it.
For now I’m reading the general things about contributing to Maven and
looking at the old branch that was merged and then reverted.
I see the master branch of Maven Resolver corresponds to v1.7.x, which
requires Maven 4, however I would love to target the current Maven 3.x.
Where should I start my PR off master or maven-resolver-1.6.x?
Thanks,
Ivan
Re: Another take on [MRESOLVER-7] Download dependency POMs in
parallel
Posted by Michael Osipov <mi...@apache.org>.
Hi Ivan,
Am 2021-10-16 um 17:21 schrieb Ivan Babiankou:
> I would love to prepare decent PR
> <https://issues.apache.org/jira/browse/MRESOLVER-7?focusedCommentId=17171271&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17171271>
> for the MRESOLVER-7 issue, so that it makes it to one of the upcoming
> releases. But I could definitely use some guidance, given the history of
> the ticket
> <https://issues.apache.org/jira/browse/MRESOLVER-7?focusedCommentId=16885618&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-16885618>
> and the fact that it’s my first contribution to Maven and OSS in general.
> Is there anyone willing to point me in the right direction (in email or a
> quick 15-30min call)? I would really appreciate it.
>
> For now I’m reading the general things about contributing to Maven and
> looking at the old branch that was merged and then reverted.
There is absolutely nothing special in providing a PR:
* Add a comment in the ticket about your intentions
* Fork repo
* Create branch based on the ticket name
* Do your magic
* Create PR
The above issue is sensible because many have tried and failed.
> I see the master branch of Maven Resolver corresponds to v1.7.x, which
> requires Maven 4, however I would love to target the current Maven 3.x.
> Where should I start my PR off master or maven-resolver-1.6.x?
Please go back a few weeks in history on the dev list. There is a
discussion about Maven 3.9.0 with Resolver 1.7+. I have no intentions to
introduce any more changes, but fixes to Resolver 1.6.x. If you are able
to provide a proper fix for MRESOLVER-7 I'd even cut Resolver 1.8.0 for
this.
Michael
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org
Re: Another take on [MRESOLVER-7] Download dependency POMs in parallel
Posted by Ivan Babiankou <iv...@gmail.com>.
Hi Tibor,
It took a while (and a few attempts :)), but I've prepared a branch
<https://github.com/ibabiankou/maven-resolver/pull/2> with a few
independent minimalistic commits. It's still a work in progress(tests and
comments are missing), but I would love to run some thoughts by you before
I spend more time on it.
The code works like a charm, I packaged a local version of Maven with these
changes and tested it on a few projects of different sizes - nothing hangs,
nothing fails.
All tests pass, when an executor with a single thread is used, but with
multiple threads there are failures because due to concurrency (and not
only) the ordering in the dependency tree is not guaranteed. All the
dependencies are there, but the order of the children might change.
I wonder what is the best way to fix those tests:
- Change assertions so that they take into account the fact that order
might differ? - not sure how easy it will be.
- Change DependencyNode to use SortedSet to store children? - IIUC it would
be a breaking change for the public API, so too big of a change, though it
would be nice to guarantee some order for the children.
- Simply sort children before returning? - Might impact performance.
- Something else?
What do you think?
Cheers,
Ivan
On Mon, Oct 18, 2021 at 11:06 AM Ivan Babiankou <iv...@gmail.com>
wrote:
> Hi Tibor,
>
> Thanks for getting back to me!
>
> I looked at the reverted changes and it seems that the majority of the
> classes used to pass around data are not really thread safe. Given the size
> of the change I'm a bit reluctant to try to fix that branch because I'm not
> sure all of the changes are needed. Instead I want to give it a try to
> refactor the `DefaultDependencyCollector.processDependency` method to be
> more functional so that it can be easily used in an executor without
> modifying passed in objects. WDYT about this approach?
>
> I'll experiment a bit and once I have something worth sharing and
> discussing I'll send an email ;)
>
> Cheers,
> Ivan
>
> On Sat, Oct 16, 2021 at 10:45 PM Tibor Digana <ti...@apache.org>
> wrote:
>
>> Hi Ivan,
>>
>> Basically the process is simple.
>> You should fork the master to your branch named, e.g. MRESOLVER-7.
>>
>> I think the process is not a big issue right now.
>> The big issue here is the analysis of the code and a proposed fix.
>>
>> It would be fine to split the changes into several independent commits or
>> (branches if possible and necessary) upon the my hints in Jira:
>> 1. root cause of hanging Maven and deadlock of thread-resources
>> starvation,
>> 2. problems with thread-unsafe classes,
>> 3. overrided objects across threads,
>> 4. one unclear part in the algorithm
>>
>> The ideal would be to isolate these four however it is a theory and still
>> keep the build success.
>>
>> You should step into 1 at first, start the code analysis, understand the
>> problem with resource starvation and propose a fix or alternatives in a
>> comment. Then we would make a decision together.
>> Then the code will be changed and the community should check it out by
>> integrating into Maven and ITs. We would be happy if we could not have any
>> regression in each step.
>>
>> If we could isolate the work in 4 or more fixes and Jira tickets, we
>> would have an easier life to bisect the commits and verify them separately
>> while we are in troubles in the future.
>>
>> T
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> On Sat, Oct 16, 2021 at 5:21 PM Ivan Babiankou <iv...@gmail.com>
>> wrote:
>>
>>> I would love to prepare decent PR
>>> <https://issues.apache.org/jira/browse/MRESOLVER-7?focusedCommentId=17171271&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17171271>
>>> for the MRESOLVER-7 issue, so that it makes it to one of the upcoming
>>> releases. But I could definitely use some guidance, given the history
>>> of the ticket
>>> <https://issues.apache.org/jira/browse/MRESOLVER-7?focusedCommentId=16885618&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-16885618>
>>> and the fact that it’s my first contribution to Maven and OSS in general.
>>> Is there anyone willing to point me in the right direction (in email or a
>>> quick 15-30min call)? I would really appreciate it.
>>>
>>> For now I’m reading the general things about contributing to Maven and
>>> looking at the old branch that was merged and then reverted.
>>>
>>>
>>> I see the master branch of Maven Resolver corresponds to v1.7.x, which
>>> requires Maven 4, however I would love to target the current Maven 3.x.
>>> Where should I start my PR off master or maven-resolver-1.6.x?
>>>
>>> Thanks,
>>> Ivan
>>>
>>>
>>>
Re: Another take on [MRESOLVER-7] Download dependency POMs in parallel
Posted by Ivan Babiankou <iv...@gmail.com>.
Hi Tibor,
Thanks for getting back to me!
I looked at the reverted changes and it seems that the majority of the
classes used to pass around data are not really thread safe. Given the size
of the change I'm a bit reluctant to try to fix that branch because I'm not
sure all of the changes are needed. Instead I want to give it a try to
refactor the `DefaultDependencyCollector.processDependency` method to be
more functional so that it can be easily used in an executor without
modifying passed in objects. WDYT about this approach?
I'll experiment a bit and once I have something worth sharing and
discussing I'll send an email ;)
Cheers,
Ivan
On Sat, Oct 16, 2021 at 10:45 PM Tibor Digana <ti...@apache.org>
wrote:
> Hi Ivan,
>
> Basically the process is simple.
> You should fork the master to your branch named, e.g. MRESOLVER-7.
>
> I think the process is not a big issue right now.
> The big issue here is the analysis of the code and a proposed fix.
>
> It would be fine to split the changes into several independent commits or
> (branches if possible and necessary) upon the my hints in Jira:
> 1. root cause of hanging Maven and deadlock of thread-resources starvation,
> 2. problems with thread-unsafe classes,
> 3. overrided objects across threads,
> 4. one unclear part in the algorithm
>
> The ideal would be to isolate these four however it is a theory and still
> keep the build success.
>
> You should step into 1 at first, start the code analysis, understand the
> problem with resource starvation and propose a fix or alternatives in a
> comment. Then we would make a decision together.
> Then the code will be changed and the community should check it out by
> integrating into Maven and ITs. We would be happy if we could not have any
> regression in each step.
>
> If we could isolate the work in 4 or more fixes and Jira tickets, we would
> have an easier life to bisect the commits and verify them separately while
> we are in troubles in the future.
>
> T
>
>
>
>
>
>
>
>
>
> On Sat, Oct 16, 2021 at 5:21 PM Ivan Babiankou <iv...@gmail.com>
> wrote:
>
>> I would love to prepare decent PR
>> <https://issues.apache.org/jira/browse/MRESOLVER-7?focusedCommentId=17171271&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17171271>
>> for the MRESOLVER-7 issue, so that it makes it to one of the upcoming
>> releases. But I could definitely use some guidance, given the history of
>> the ticket
>> <https://issues.apache.org/jira/browse/MRESOLVER-7?focusedCommentId=16885618&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-16885618>
>> and the fact that it’s my first contribution to Maven and OSS in general.
>> Is there anyone willing to point me in the right direction (in email or a
>> quick 15-30min call)? I would really appreciate it.
>>
>> For now I’m reading the general things about contributing to Maven and
>> looking at the old branch that was merged and then reverted.
>>
>>
>> I see the master branch of Maven Resolver corresponds to v1.7.x, which
>> requires Maven 4, however I would love to target the current Maven 3.x.
>> Where should I start my PR off master or maven-resolver-1.6.x?
>>
>> Thanks,
>> Ivan
>>
>>
>>
Re: Another take on [MRESOLVER-7] Download dependency POMs in parallel
Posted by Tibor Digana <ti...@apache.org>.
Hi Ivan,
Basically the process is simple.
You should fork the master to your branch named, e.g. MRESOLVER-7.
I think the process is not a big issue right now.
The big issue here is the analysis of the code and a proposed fix.
It would be fine to split the changes into several independent commits or
(branches if possible and necessary) upon the my hints in Jira:
1. root cause of hanging Maven and deadlock of thread-resources starvation,
2. problems with thread-unsafe classes,
3. overrided objects across threads,
4. one unclear part in the algorithm
The ideal would be to isolate these four however it is a theory and still
keep the build success.
You should step into 1 at first, start the code analysis, understand the
problem with resource starvation and propose a fix or alternatives in a
comment. Then we would make a decision together.
Then the code will be changed and the community should check it out by
integrating into Maven and ITs. We would be happy if we could not have any
regression in each step.
If we could isolate the work in 4 or more fixes and Jira tickets, we would
have an easier life to bisect the commits and verify them separately while
we are in troubles in the future.
T
On Sat, Oct 16, 2021 at 5:21 PM Ivan Babiankou <iv...@gmail.com>
wrote:
> I would love to prepare decent PR
> <https://issues.apache.org/jira/browse/MRESOLVER-7?focusedCommentId=17171271&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17171271>
> for the MRESOLVER-7 issue, so that it makes it to one of the upcoming
> releases. But I could definitely use some guidance, given the history of
> the ticket
> <https://issues.apache.org/jira/browse/MRESOLVER-7?focusedCommentId=16885618&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-16885618>
> and the fact that it’s my first contribution to Maven and OSS in general.
> Is there anyone willing to point me in the right direction (in email or a
> quick 15-30min call)? I would really appreciate it.
>
> For now I’m reading the general things about contributing to Maven and
> looking at the old branch that was merged and then reverted.
>
>
> I see the master branch of Maven Resolver corresponds to v1.7.x, which
> requires Maven 4, however I would love to target the current Maven 3.x.
> Where should I start my PR off master or maven-resolver-1.6.x?
>
> Thanks,
> Ivan
>
>
>