ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andrey Mashenkov <andrey.mashen...@gmail.com>
Subject Re: Code inspection
Date Thu, 13 Dec 2018 09:54:16 GMT
Guys,

I've just creates a copy of Inspections TC build task with GC logs turned
on to check if there is any issues
and found Inspections task spent too much time in STW due to long Full GC
pauses.

I've tried to increase Xmx up to 4Gb and use G1GC got 2+ times better
execution time down to ~15 min (~17 for 2G heap).
Increasing heap size only is not very helpful as it just postpone Full GC
issues, but changing GC to G1GC gives noticeable result.

Let's apply this optimization.
Thoughts?


On Sun, Dec 9, 2018 at 12:43 PM Vyacheslav Daradur <daradurvs@gmail.com>
wrote:

> Hi, Maxim, Nikolay, I have the following questions regarding inspections:
>
> Should 'gnite_inspections_teamcity.xml' been imported into IDEA, since
> 'ignite_inspections.xml' has been removed in actual master?
>
> Also, I've faced mismatching: if I use
> '@SuppressWarnings("ErrorNotRethrown")' in code, then this will be
> marked on TC as "Redundant suppression". If I removed this suppression
> in "main" code base (not in tests) then it's fine and IDE does not
> mark the code by inspection. But, if I use
> 'GridTestUtils#assertThrows' in 'tests' code base, then IDE requires
> to suppress the inspection, if I have done it then TC marks this as
> "Redundant suppression".
>
> What should I do in this case?
>
> On Mon, Dec 3, 2018 at 10:26 PM Andrey Mashenkov
> <andrey.mashenkov@gmail.com> wrote:
> >
> > Hi,
> >
> > Have someone tried to investigate the issue related to Inspection TC task
> > execution time variation (from 0.5 up to 1,5 hours)?
> > Can we enable GC logs for this task or may be even get CPU, Disk, Network
> > metrics?
> > Can someone check if there are unnecessary Idea plugins starts that can
> be
> > safely disabled?
> >
> >
> > On Tue, Nov 27, 2018 at 5:52 PM Dmitriy Pavlov <dpavlov@apache.org>
> wrote:
> >
> > > I'm totally with you in this decision, let's move the file.
> > >
> > > вт, 27 нояб. 2018 г. в 16:24, Maxim Muzafarov <maxmuzaf@gmail.com>:
> > >
> > > > Igniters,
> > > >
> > > > I propose to make inspection configuration default on the project
> > > > level. I've created a new issue [1] for it. It can be easily done and
> > > > recommend by IntelliJ documentation [2].
> > > > Thoughts?
> > > >
> > > >
> > > > Vyacheslav,
> > > >
> > > > Can you share an example of your warnings?
> > > > Currently, we have different inspection configurations:
> > > > - ignite_inspections.xml - to import inspections as default and use
> it
> > > > daily.
> > > > - ignite_inspections_teamcity.xml - config to run it on TC. Only
> fixed
> > > > rules in the project code are enabled. Each of these rules are marked
> > > > with ERROR level.
> > > >
> > > > [1] https://issues.apache.org/jira/browse/IGNITE-10422
> > > > [2] https://www.jetbrains.com/help/idea/code-inspection.html
> > > > On Tue, 20 Nov 2018 at 13:58, Nikolay Izhikov <nizhikov@apache.org>
> > > wrote:
> > > > >
> > > > > Hello, Vyacheslav.
> > > > >
> > > > > Yes, we have.
> > > > >
> > > > > Maxim Muzafarov, can you fix it, please?
> > > > >
> > > > > вт, 20 нояб. 2018 г., 13:10 Vyacheslav Daradur daradurvs@gmail.com
> :
> > > > >
> > > > > > Guys, why we have 2 different inspection files in the repo?
> > > > > > idea\ignite_inspections.xml
> > > > > > idea\ignite_inspections_teamcity.xml
> > > > > >
> > > > > > AFAIK TeamCity is able to use the same inspection file with
IDE.
> > > > > >
> > > > > > I've imported 'idea\ignite_inspections.xml' in the IDE, but
now
> see
> > > > > > inspection warnings for my PR on TC because of different rules.
> > > > > >
> > > > > >
> > > > > > On Sun, Nov 11, 2018 at 6:06 PM Maxim Muzafarov <
> maxmuzaf@gmail.com>
> > > > > > wrote:
> > > > > > >
> > > > > > > Yakov, Dmitry,
> > > > > > >
> > > > > > > Which example of unsuccessful suite execution do we need?
> > > > > > > Does the current fail [1] in the master branch enough to
> configure
> > > > > > > notifications by TC.Bot?
> > > > > > >
> > > > > > > > Please consider adding more checks
> > > > > > > > - line endings. I think we should only have \n
> > > > > > > > - ensure blank line at the end of file
> > > > > > >
> > > > > > > It seems to me that `line endings` is easy to add, but
for the
> > > `blank
> > > > > > > line at the end` we need as special regexp. Can we focus
on
> > > built-in
> > > > > > > IntelliJ inspections at first and fix others special further?
> > > > > > >
> > > > > > > [1]
> > > > > >
> > > >
> > >
> https://ci.ignite.apache.org/viewType.html?buildTypeId=IgniteTests24Java8_InspectionsCore&branch_IgniteTests24Java8=%3Cdefault%3E&tab=buildTypeStatusDiv
> > > > > > > On Sun, 11 Nov 2018 at 17:55, Maxim Muzafarov <
> maxmuzaf@gmail.com>
> > > > > > wrote:
> > > > > > > >
> > > > > > > > Igniters,
> > > > > > > >
> > > > > > > > Since the inspection rules are included in RunAll
a few
> members
> > > of
> > > > the
> > > > > > > > community mentioned a wide distributed execution time
on TC
> > > agents:
> > > > > > > >  - 1h:27m:38s publicagent17_9094
> > > > > > > >  - 38m:04s publicagent17_9094
> > > > > > > >  - 33m:29s publicagent17_9094
> > > > > > > >  - 17m:13s publicagent17_9094
> > > > > > > > It seems that we should configure the resources distribution
> > > > across TC
> > > > > > > > containers. Can anyone take a look at it?
> > > > > > > >
> > > > > > > >
> > > > > > > > I've also prepared the short list of rules to work
on:
> > > > > > > > + Inconsistent line separators (6 matches)
> > > > > > > > + Problematic whitespace (4 matches)
> > > > > > > > + expression.equals("literal")' rather than
> > > > > > > > '"literal".equals(expression) (53 matches)
> > > > > > > > + Unnecessary 'null' check before 'instanceof' expression
or
> call
> > > > (42
> > > > > > matches)
> > > > > > > > + Redundant 'if' statement (69 matches)
> > > > > > > > + Redundant interface declaration (28 matches)
> > > > > > > > + Double negation (0 matches)
> > > > > > > > + Unnecessary code block (472 matches)
> > > > > > > > + Line is longer than allowed by code style (2614
matches)
> (Is it
> > > > > > > > possible to implement?)
> > > > > > > >
> > > > > > > > WDYT?
> > > > > > > >
> > > > > > > > On Fri, 26 Oct 2018 at 23:43, Dmitriy Pavlov <
> > > > dpavlov.spb@gmail.com>
> > > > > > wrote:
> > > > > > > > >
> > > > > > > > > Hi Maxim,
> > > > > > > > >
> > > > > > > > >  thank you for your efforts to make this happen.
Keep the
> pace!
> > > > > > > > >
> > > > > > > > > Could you please provide an example of how Inspections
can
> > > fail,
> > > > so
> > > > > > I or
> > > > > > > > > another contributor could implement support of
these
> failures
> > > > > > validation in
> > > > > > > > > the Tc Bot.
> > > > > > > > >
> > > > > > > > > Sincerely,
> > > > > > > > > Dmitriy Pavlov
> > > > > > > > >
> > > > > > > > > пт, 26 окт. 2018 г. в 18:27, Yakov Zhdanov
<
> > > yzhdanov@apache.org
> > > > >:
> > > > > > > > >
> > > > > > > > > > Maxim,
> > > > > > > > > >
> > > > > > > > > > Thanks for response, let's do it the way
you suggested.
> > > > > > > > > >
> > > > > > > > > > Please consider adding more checks
> > > > > > > > > > - line endings. I think we should only have
\n
> > > > > > > > > > - ensure blank line in the end of file
> > > > > > > > > >
> > > > > > > > > > All these are code reviews issues I pointed
out many
> times
> > > when
> > > > > > reviewing
> > > > > > > > > > conributions. It would be cool if we have
TC build
> failing if
> > > > > > there is any.
> > > > > > > > > >
> > > > > > > > > > Thanks!
> > > > > > > > > >
> > > > > > > > > > --Yakov
> > > > > > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > > --
> > > > > > Best Regards, Vyacheslav D.
> > > > > >
> > > >
> > >
> >
> >
> > --
> > Best regards,
> > Andrey V. Mashenkov
>
>
>
> --
> Best Regards, Vyacheslav D.
>


-- 
Best regards,
Andrey V. Mashenkov

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message