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 Fri, 14 Dec 2018 09:00:07 GMT
Maxim,

Thanks, I'll check PR and let you know about results.

For now, Inspections task execution time looks much better (15-22 min), but
fluctuation is still noticeable.

On Fri, Dec 14, 2018 at 11:13 AM Maxim Muzafarov <maxmuzaf@gmail.com> wrote:

> Andrey,
>
> Thanks! I've consulted with the IntelliJ IDEA source code and found
> how this disabled plugins file should look like. I've created a new
> issue [1] and prepared PR [2] with the set of disabled plugins (maybe
> not complete set). I don't have access to change corresponding
> `~Excluded [Inspections] Core Debug` test suite properties.
> Can we test this PR?
>
> [1] https://issues.apache.org/jira/browse/IGNITE-10682
> [2] https://github.com/apache/ignite/pull/5666
> On Thu, 13 Dec 2018 at 17:35, Andrey Mashenkov
> <andrey.mashenkov@gmail.com> wrote:
> >
> > Maxim,
> >
> > Idea has a file in config directory ./config/disabled_plugins.txt , you
> can easily find it at you local machine.
> > Teamcity Inspections runner has an option "Disabled plugins" where
> disabled_plugins.txt file content can be set.
> >
> > So, looks like we can disable useless plugins.
> > But I'm not expert and can't suggest changes we can safely apply.
> >
> > On Thu, Dec 13, 2018 at 4:59 PM Maxim Muzafarov <maxmuzaf@gmail.com>
> wrote:
> >>
> >> Andrey,
> >>
> >> Thank you for solving this issue with GC pauses! I've checked the
> >> given report. The inspections configuration is correct, but it seems
> >> to me that we have enabled by default rules of included plugins (for
> >> instance, KotlinInternalInJava in the report is enabled).
> >>
> >> Can you share more details about `disable plugin` option you found?
> >>
> >> I see that idea instance starts with the default -Didea.plugins.path
> >> system property, can we change it so the plugins will be not loaded by
> >> default?
> >> On Thu, 13 Dec 2018 at 15:45, Andrey Mashenkov
> >> <andrey.mashenkov@gmail.com> wrote:
> >> >
> >> > Maxim,
> >> >
> >> > It looks like we can't make logs more verbose due to possible bug,
> I've create a ticket in Jetbrains Jira [1].
> >> > We can just publish idea logs in artefacts as suggested in this
> manual [2].
> >> >
> >> > For now, Inspections logs looks like this one [3].
> >> > Also, would you please to take a look at inspection report and check
> if we missed smth and there are any unwanted inspection turned on.
> >> >
> >> > [1] https://youtrack.jetbrains.com/issue/TW-58422
> >> > [2]
> https://confluence.jetbrains.com/display/TCD10/Reporting+Issues#ReportingIssues-IntelliJIDEAInspections
> >> > [3]
> https://ci.ignite.apache.org/viewLog.html?buildId=2538111&buildTypeId=IgniteTests24Java8_ExcludedInspections2&tab=artifacts
> >> >
> >> > On Thu, Dec 13, 2018 at 3:19 PM Dmitriy Pavlov <dpavlov@apache.org>
> wrote:
> >> >>
> >> >> Maxim M, do you know if we can disable inspections by wildcard? E.g.
> >> >> Android* ?
> >> >>
> >> >> чт, 13 дек. 2018 г. в 14:59, Andrey Mashenkov <
> andrey.mashenkov@gmail.com>:
> >> >>
> >> >> > Fixed memory issues with increasing heap size and forcing G1GC.
> >> >> >
> >> >> > Do we need all these plugins loaded for inspections?
> >> >> > I've found a 'disable plugin' option in TC Inspections build
> configuration,
> >> >> > but it is unclear how to disable plugin correctly.
> >> >> > Can someone take over this?
> >> >> >
> >> >> > > 46 plugins initialized in 1031 ms
> >> >> > > 2018-12-13 10:55:24,875 [ 1342] INFO -
> llij.ide.plugins.PluginManager -
> >> >> > > Loaded bundled plugins: Android Support (10.2.3), Ant Support
> (1.0), CSS
> >> >> > > Support (172.4574.11), Database Tools and SQL (172.4574.11),
> Eclipse
> >> >> > > Integration (3.0), FreeMarker support (1.0), GWT Support
(1.0),
> Gradle
> >> >> > > (172.4574.11), Groovy (9.0), Guice (8.0), HTML Tools (2.0),
> Hibernate
> >> >> > > Support (1.0), I18n for Java (172.4574.11), IDEA CORE
> (172.4574.11),
> >> >> > > IntelliLang (8.0), JBoss Seam Support (1.0), JUnit (1.0),
Java
> EE: Bean
> >> >> > > Validation Support (1.1), Java EE: Contexts and Dependency
> Injection
> >> >> > (1.1),
> >> >> > > Java EE: EJB, JPA, Servlets (1.0), Java EE: Java Server Faces
> (2.2.X.),
> >> >> > > Java EE: Web Services (JAX-WS) (1.9), Java Server Pages (JSP)
> Integration
> >> >> > > (1.0), JavaScript Support (1.0), Kotlin
> (1.1.4-release-IJ2017.2-3), Maven
> >> >> > > Integration (172.4574.11), Persistence Frameworks Support
(1.0),
> Plugin
> >> >> > > DevKit (1.0), Properties Support (172.4574.11), QuirksMode
> (172.4574.11),
> >> >> > > Spring AOP/@AspectJ (1.0), Spring Batch (1.0), Spring Data
> (1.0), Spring
> >> >> > > Integration Patterns (1.0), Spring Security (1.0), Spring
> Support (1.0),
> >> >> > > Spring Web Flow (1.0), Spring Web Services (1.0), Struts
1.x
> (2.0),
> >> >> > Struts
> >> >> > > 2 (1.0), TestNG-J (8.0), UI Designer (172.4574.11), Velocity
> support
> >> >> > (1.0),
> >> >> > > W3C Validators (2.0), WebLogic Integration (1.0), XPathView
+
> XSLT
> >> >> > Support
> >> >> > > (4)
> >> >> >
> >> >> >
> >> >> > Kotlin plugins fails to start, let's disable it.
> >> >> >
> >> >> > >
> >> >> > > 2018-12-13 10:55:27,623 [   4090]   INFO -
> >> >> > il.indexing.FileBasedIndexImpl - Rebuild requested for index
> >> >> > org.jetbrains.kotlin.idea.versions.KotlinJvmMetadataVersionIndex
> >> >> > > java.lang.Throwable
> >> >> > >       at
> >> >> >
> com.intellij.util.indexing.FileBasedIndex.requestRebuild(FileBasedIndex.java:68)
> >> >> > >       at
> >> >> >
> org.jetbrains.kotlin.idea.versions.KotlinUpdatePluginComponent.initComponent(KotlinUpdatePluginComponent.kt:54)
> >> >> > >       at
> >> >> >
> com.intellij.openapi.components.impl.ComponentManagerImpl$ComponentConfigComponentAdapter.getComponentInstance(ComponentManagerImpl.java:492)
> >> >> > >       at
> >> >> >
> com.intellij.openapi.components.impl.ComponentManagerImpl.createComponents(ComponentManagerImpl.java:118)
> >> >> > >       at
> >> >> >
> com.intellij.openapi.application.impl.ApplicationImpl.a(ApplicationImpl.java:462)
> >> >> > >       at
> >> >> >
> com.intellij.openapi.application.impl.ApplicationImpl.createComponents(ApplicationImpl.java:466)
> >> >> > >       at
> >> >> >
> com.intellij.openapi.components.impl.ComponentManagerImpl.init(ComponentManagerImpl.java:102)
> >> >> > >       at
> >> >> >
> com.intellij.openapi.application.impl.ApplicationImpl.load(ApplicationImpl.java:421)
> >> >> > >       at
> >> >> >
> com.intellij.openapi.application.impl.ApplicationImpl.load(ApplicationImpl.java:407)
> >> >> > >       at
> com.intellij.idea.IdeaApplication.run(IdeaApplication.java:203)
> >> >> >
> >> >> >
> >> >> >
> >> >> > On Thu, Dec 13, 2018 at 1:45 PM Dmitriy Pavlov <dpavlov@apache.org>
> wrote:
> >> >> >
> >> >> > > Sure, let's apply. I hope all TC agents may handle 4G heap.
> >> >> > >
> >> >> > > чт, 13 дек. 2018 г. в 12:54, Andrey Mashenkov <
> >> >> > andrey.mashenkov@gmail.com
> >> >> > > >:
> >> >> > >
> >> >> > > > 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
> >> >> > > >
> >> >> > >
> >> >> >
> >> >> >
> >> >> > --
> >> >> > Best regards,
> >> >> > Andrey V. Mashenkov
> >> >> >
> >> >
> >> >
> >> >
> >> > --
> >> > Best regards,
> >> > Andrey V. Mashenkov
> >
> >
> >
> > --
> > Best regards,
> > Andrey V. Mashenkov
>


-- 
Best regards,
Andrey V. Mashenkov

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