Re: Instructions for Setting Up IntelliJ

[Michael Oleske]

+1 for the repo as source of truth. Wiki always seemed better as a place to
show examples rather than how to build the project.  I also like having the
readme about how to use the product and the building about how to manually
build so one could contribute (especially since Geode already provided
(unofficial) convenience binaries)

-michael

On Tuesday, September 11, 2018, Alexander Murmann 
wrote:

> +1 for putting it into the repo. I am fine with either putting it into
> README.md or linking form there.
>
> On Tue, Sep 11, 2018 at 4:33 PM, Ryan McMahon 
> wrote:
>
> > Kirk - I have a PR open here which has the "Setting up IntelliJ" section
> > you've described:
> > https://github.com/apache/geode/pull/2456
> >
> > I would be happy to use your version if you think it is more
> comprehensive,
> > or we can revise my PR to include any details you feel are missing.
> >
> > Ryan
> >
> > On Tue, Sep 11, 2018 at 4:15 PM, Jacob Barrett 
> > wrote:
> >
> > > Put it with the source! BUILDING.md or other file.
> > >
> > > > On Sep 11, 2018, at 4:11 PM, Kirk Lund  wrote:
> > > >
> > > > I don't care which location (wiki or part of the readme) but I do
> have
> > > > up-to-date instructions that I could update either location with.
> > Maybe a
> > > > detailed "Setting up IntelliJ" section on BUILDING.md? Just let me
> know
> > > if
> > > > you'd like my version.
> > > >
> > > >> On Tue, Sep 11, 2018 at 3:36 PM, Jianxia Chen 
> > > wrote:
> > > >>
> > > >> +1 to revise the wiki to link to README.md/BUILDING.md
> > > >>
> > > >> On Tue, Sep 11, 2018 at 3:22 PM Ryan McMahon <
> mcmellaw...@apache.org>
> > > >> wrote:
> > > >>
> > > >>> Hi all,
> > > >>>
> > > >>> I am looking to add more comprehensive instructions for how to
> bring
> > > >> Geode
> > > >>> into IntelliJ.  I've written the instructions but am now looking at
> > > where
> > > >>> to put them.
> > > >>>
> > > >>> There appears to be duplicate information in these sections of the
> > > Geode
> > > >>> wiki and the README.md/BUILDING.md in the Geode Git repository.
> > > >>>
> > > >>>
> > > >>> https://cwiki.apache.org/confluence/display/GEODE/
> > > >> Getting+Started+for+Geode+Developers
> > > >>>
> > > >>> https://cwiki.apache.org/confluence/display/GEODE/
> > > >> Building+and+Running+Geode+from+Source
> > > >>>
> > > >>> I'm a fan of one source of truth, but I wanted to see if anyone
> felt
> > > >>> strongly about where the information lives.  I propose we revise
> the
> > > wiki
> > > >>> to simply link to the README.md/BUILDING.md and eliminate the
> > duplicate
> > > >>> information (how to run gradlew, etc).  Any thoughts?
> > > >>>
> > > >>> Thanks,
> > > >>> Ryan
> > > >>>
> > > >>
> > >
> >
>

Re: Instructions for Setting Up IntelliJ

[Alexander Murmann]

+1 for putting it into the repo. I am fine with either putting it into
README.md or linking form there.

On Tue, Sep 11, 2018 at 4:33 PM, Ryan McMahon 
wrote:

> Kirk - I have a PR open here which has the "Setting up IntelliJ" section
> you've described:
> https://github.com/apache/geode/pull/2456
>
> I would be happy to use your version if you think it is more comprehensive,
> or we can revise my PR to include any details you feel are missing.
>
> Ryan
>
> On Tue, Sep 11, 2018 at 4:15 PM, Jacob Barrett 
> wrote:
>
> > Put it with the source! BUILDING.md or other file.
> >
> > > On Sep 11, 2018, at 4:11 PM, Kirk Lund  wrote:
> > >
> > > I don't care which location (wiki or part of the readme) but I do have
> > > up-to-date instructions that I could update either location with.
> Maybe a
> > > detailed "Setting up IntelliJ" section on BUILDING.md? Just let me know
> > if
> > > you'd like my version.
> > >
> > >> On Tue, Sep 11, 2018 at 3:36 PM, Jianxia Chen 
> > wrote:
> > >>
> > >> +1 to revise the wiki to link to README.md/BUILDING.md
> > >>
> > >> On Tue, Sep 11, 2018 at 3:22 PM Ryan McMahon 
> > >> wrote:
> > >>
> > >>> Hi all,
> > >>>
> > >>> I am looking to add more comprehensive instructions for how to bring
> > >> Geode
> > >>> into IntelliJ.  I've written the instructions but am now looking at
> > where
> > >>> to put them.
> > >>>
> > >>> There appears to be duplicate information in these sections of the
> > Geode
> > >>> wiki and the README.md/BUILDING.md in the Geode Git repository.
> > >>>
> > >>>
> > >>> https://cwiki.apache.org/confluence/display/GEODE/
> > >> Getting+Started+for+Geode+Developers
> > >>>
> > >>> https://cwiki.apache.org/confluence/display/GEODE/
> > >> Building+and+Running+Geode+from+Source
> > >>>
> > >>> I'm a fan of one source of truth, but I wanted to see if anyone felt
> > >>> strongly about where the information lives.  I propose we revise the
> > wiki
> > >>> to simply link to the README.md/BUILDING.md and eliminate the
> duplicate
> > >>> information (how to run gradlew, etc).  Any thoughts?
> > >>>
> > >>> Thanks,
> > >>> Ryan
> > >>>
> > >>
> >
>

Re: [DISCUSS] and the NEW Apache Geode 1.7.0 release branch has been created

[Anthony Baker]

Slight clarification—the issue I mentioned is when a user builds Geode from the 
source distribution.  The source distribution that the release manager creates 
has the correct .buildinfo file, it’s just ignored by the build.  In short, the 
release manager can’t work around the problem.

Does [1] help with this?

Anthony

[1] https://github.com/apache/geode/pull/2457 



> On Sep 11, 2018, at 3:16 PM, Alexander Murmann  wrote:
> 
> What's the consensus on the version info issue Anthony is calling out? Does
> anyone have a proposal for fixing this for this release? Should Nabarun as
> the release manager manually correct this for the release and we find a
> permanent solution for 1.8?
> 
> On Mon, Sep 10, 2018 at 12:33 PM, Anthony Baker  wrote:
> 
>> Unfortunately it would require a fix to the build—it’s not about producing
>> the release candidate. It’s when a user builds from the source release that
>> the version info is ignored.
>> 
>> Anthony
>> 
>>> On Sep 10, 2018, at 10:02 AM, Nabarun Nag  wrote:
>>> 
>>> Hello Anthony,
>>> 
>>> I plan to do that while creating the release candidate. If there are no
>>> concerns raised on the release branch, I will start with the process
>> soon.
>>> 
>>> Regards
>>> Nabarun Nag
>>> 
 On Mon, Sep 10, 2018 at 8:51 AM Anthony Baker 
>> wrote:
 
 Looks good Naba!  Only thing I see right now is that building from the
 source distribution does not use the .buildinfo file, leaving the
>> version
 string empty.
 
 Anthony
 
 
> On Sep 7, 2018, at 9:15 AM, Nabarun Nag  wrote:
> 
> CORRECTION: if '*no*' concerns are raised, we will start with the
>> voting
> for the release candidate soon.
> 
> Regrads
> Nabarun Nag
> 
>> On Fri, Sep 7, 2018 at 9:08 AM Nabarun Nag  wrote:
>> 
>> Hello Geode Dev Community,
>> 
>> We have created a new release branch for Apache Geode 1.7.0 -
>> "release/1.7.0"
>> 
>> Previous branch was deleted and has been replaced with a fresh one.
>> 
>> Please do review and raise any concern with the release branch.
>> 
>> If concerns are raised, we will start with the voting for the release
>> candidate soon.
>> 
>> Regards
>> Nabarun Nag
>> 
 
 --
>>> Regards
>>> Nabarun Nag
>> 

Re: Instructions for Setting Up IntelliJ

[Ryan McMahon]

Kirk - I have a PR open here which has the "Setting up IntelliJ" section
you've described:
https://github.com/apache/geode/pull/2456

I would be happy to use your version if you think it is more comprehensive,
or we can revise my PR to include any details you feel are missing.

Ryan

On Tue, Sep 11, 2018 at 4:15 PM, Jacob Barrett  wrote:

> Put it with the source! BUILDING.md or other file.
>
> > On Sep 11, 2018, at 4:11 PM, Kirk Lund  wrote:
> >
> > I don't care which location (wiki or part of the readme) but I do have
> > up-to-date instructions that I could update either location with. Maybe a
> > detailed "Setting up IntelliJ" section on BUILDING.md? Just let me know
> if
> > you'd like my version.
> >
> >> On Tue, Sep 11, 2018 at 3:36 PM, Jianxia Chen 
> wrote:
> >>
> >> +1 to revise the wiki to link to README.md/BUILDING.md
> >>
> >> On Tue, Sep 11, 2018 at 3:22 PM Ryan McMahon 
> >> wrote:
> >>
> >>> Hi all,
> >>>
> >>> I am looking to add more comprehensive instructions for how to bring
> >> Geode
> >>> into IntelliJ.  I've written the instructions but am now looking at
> where
> >>> to put them.
> >>>
> >>> There appears to be duplicate information in these sections of the
> Geode
> >>> wiki and the README.md/BUILDING.md in the Geode Git repository.
> >>>
> >>>
> >>> https://cwiki.apache.org/confluence/display/GEODE/
> >> Getting+Started+for+Geode+Developers
> >>>
> >>> https://cwiki.apache.org/confluence/display/GEODE/
> >> Building+and+Running+Geode+from+Source
> >>>
> >>> I'm a fan of one source of truth, but I wanted to see if anyone felt
> >>> strongly about where the information lives.  I propose we revise the
> wiki
> >>> to simply link to the README.md/BUILDING.md and eliminate the duplicate
> >>> information (how to run gradlew, etc).  Any thoughts?
> >>>
> >>> Thanks,
> >>> Ryan
> >>>
> >>
>

Re: Instructions for Setting Up IntelliJ

[Jacob Barrett]

Put it with the source! BUILDING.md or other file. 

> On Sep 11, 2018, at 4:11 PM, Kirk Lund  wrote:
> 
> I don't care which location (wiki or part of the readme) but I do have
> up-to-date instructions that I could update either location with. Maybe a
> detailed "Setting up IntelliJ" section on BUILDING.md? Just let me know if
> you'd like my version.
> 
>> On Tue, Sep 11, 2018 at 3:36 PM, Jianxia Chen  wrote:
>> 
>> +1 to revise the wiki to link to README.md/BUILDING.md
>> 
>> On Tue, Sep 11, 2018 at 3:22 PM Ryan McMahon 
>> wrote:
>> 
>>> Hi all,
>>> 
>>> I am looking to add more comprehensive instructions for how to bring
>> Geode
>>> into IntelliJ.  I've written the instructions but am now looking at where
>>> to put them.
>>> 
>>> There appears to be duplicate information in these sections of the Geode
>>> wiki and the README.md/BUILDING.md in the Geode Git repository.
>>> 
>>> 
>>> https://cwiki.apache.org/confluence/display/GEODE/
>> Getting+Started+for+Geode+Developers
>>> 
>>> https://cwiki.apache.org/confluence/display/GEODE/
>> Building+and+Running+Geode+from+Source
>>> 
>>> I'm a fan of one source of truth, but I wanted to see if anyone felt
>>> strongly about where the information lives.  I propose we revise the wiki
>>> to simply link to the README.md/BUILDING.md and eliminate the duplicate
>>> information (how to run gradlew, etc).  Any thoughts?
>>> 
>>> Thanks,
>>> Ryan
>>> 
>> 

Re: Instructions for Setting Up IntelliJ

[Kirk Lund]

I don't care which location (wiki or part of the readme) but I do have
up-to-date instructions that I could update either location with. Maybe a
detailed "Setting up IntelliJ" section on BUILDING.md? Just let me know if
you'd like my version.

On Tue, Sep 11, 2018 at 3:36 PM, Jianxia Chen  wrote:

> +1 to revise the wiki to link to README.md/BUILDING.md
>
> On Tue, Sep 11, 2018 at 3:22 PM Ryan McMahon 
> wrote:
>
> > Hi all,
> >
> > I am looking to add more comprehensive instructions for how to bring
> Geode
> > into IntelliJ.  I've written the instructions but am now looking at where
> > to put them.
> >
> > There appears to be duplicate information in these sections of the Geode
> > wiki and the README.md/BUILDING.md in the Geode Git repository.
> >
> >
> > https://cwiki.apache.org/confluence/display/GEODE/
> Getting+Started+for+Geode+Developers
> >
> > https://cwiki.apache.org/confluence/display/GEODE/
> Building+and+Running+Geode+from+Source
> >
> > I'm a fan of one source of truth, but I wanted to see if anyone felt
> > strongly about where the information lives.  I propose we revise the wiki
> > to simply link to the README.md/BUILDING.md and eliminate the duplicate
> > information (how to run gradlew, etc).  Any thoughts?
> >
> > Thanks,
> > Ryan
> >
>

[Spring CI] Spring Data GemFire > Nightly-ApacheGeode > #1037 was SUCCESSFUL (with 2456 tests). Change made by John Blum.

[Spring CI]

---
Spring Data GemFire > Nightly-ApacheGeode > #1037 was successful.
---
Scheduled with changes by John Blum.
2458 tests in total.

https://build.spring.io/browse/SGF-NAG-1037/




--
Code Changes
--
John Blum (055a777ef207265fbfa595b3c498b31945add42b):

>DATAGEODE-144 - Update Javadoc and Reference Guide.

John Blum (77399589690f91176699d6ea14bdef99730e553f):

>DATAGEODE-144 - Add 'requiredPermissions' to @GemfireFunction annotation.



--
This message is automatically generated by Atlassian Bamboo

Re: Instructions for Setting Up IntelliJ

[Jianxia Chen]

+1 to revise the wiki to link to README.md/BUILDING.md

On Tue, Sep 11, 2018 at 3:22 PM Ryan McMahon  wrote:

> Hi all,
>
> I am looking to add more comprehensive instructions for how to bring Geode
> into IntelliJ.  I've written the instructions but am now looking at where
> to put them.
>
> There appears to be duplicate information in these sections of the Geode
> wiki and the README.md/BUILDING.md in the Geode Git repository.
>
>
> https://cwiki.apache.org/confluence/display/GEODE/Getting+Started+for+Geode+Developers
>
> https://cwiki.apache.org/confluence/display/GEODE/Building+and+Running+Geode+from+Source
>
> I'm a fan of one source of truth, but I wanted to see if anyone felt
> strongly about where the information lives.  I propose we revise the wiki
> to simply link to the README.md/BUILDING.md and eliminate the duplicate
> information (how to run gradlew, etc).  Any thoughts?
>
> Thanks,
> Ryan
>

Instructions for Setting Up IntelliJ

[Ryan McMahon]

Hi all,

I am looking to add more comprehensive instructions for how to bring Geode
into IntelliJ.  I've written the instructions but am now looking at where
to put them.

There appears to be duplicate information in these sections of the Geode
wiki and the README.md/BUILDING.md in the Geode Git repository.

https://cwiki.apache.org/confluence/display/GEODE/Getting+Started+for+Geode+Developers
https://cwiki.apache.org/confluence/display/GEODE/Building+and+Running+Geode+from+Source

I'm a fan of one source of truth, but I wanted to see if anyone felt
strongly about where the information lives.  I propose we revise the wiki
to simply link to the README.md/BUILDING.md and eliminate the duplicate
information (how to run gradlew, etc).  Any thoughts?

Thanks,
Ryan

Re: [DISCUSS] Wrapping log calls in Conditionals like isDebugEnabled, isTraceEnabled, etc.

[Kirk Lund]

Here is AsyncEventListenerDUnitTest that (latest versions of) IntelliJ and
Eclipse both complained exceeded the bytes limit for lambdas:

https://github.com/apache/geode/blob/6ef68c11fdd49e02ded0bfa2bfc072f96f7ab250/geode-core/src/distributedTest/java/org/apache/geode/internal/cache/wan/asyncqueue/AsyncEventListenerDUnitTest.java

I don't think it's a limit based on "count" but rather something to do with
byte size of the lambdas. This never failed to compile. And yes, I read up
on the same SO post when I discovered the problem in this file but that
explain it clearly to me within the context of AsyncEventListenerDUnitTest.
The JIRA ticket I filed was GEODE-5485 "AsyncEventListenerDUnitTest
$deserializeLambda$(SerializedLambda) is exceeding the 65535 bytes limit"

On Tue, Sep 11, 2018 at 9:26 AM, John Blum  wrote:

> I think any arguments about what optimizations the (JIT enabled) compiler
> (HotSpot or otherwise) will perform at runtime is questionable at best.
> HotSpot can "inline" certain [frequent/hot] code paths both at
> compile/runtime thereby reducing the number of method invocations (which
> also depends on many factors, not the least of which is method size).
>
> Here is a good SO post on the subject of Lambdas...
>
> https://stackoverflow.com/questions/40860859/what-is-
> the-maximum-number-of-lambdas-used-in-one-java-class
>
> I find it somewhat surprising that Geode (in any part of its codebase) hit
> the limit on the number of Lambdas (methods).  That suggests that class
> themselves need to be refactored and better organized by concern.
>
>
> On Tue, Sep 11, 2018 at 9:02 AM, Dale Emery  wrote:
>
> > If there’s enough duplication in the lambdas, or in the code around the
> > lambdas, extracting the duplication into methods would reduce the number
> of
> > lambdas.
> >
> > > On Sep 10, 2018, at 11:03 AM, Kirk Lund  wrote:
> > >
> > > Alright I'm more up-to-date on this thread. Some things to consider:
> > >
> > > The lambdas look great, but we'd have to start splitting the Geode
> > clasess.
> > > They're so big right now, that if you change a bunch of log statements
> to
> > > use lambdas you'll hit the max number of lambdas on many of our
> classes.
> > We
> > > hit the lambda limit on a dunit test and it didn't really take that
> many
> > > lambdas to hit the limit (see
> > > https://issues.apache.org/jira/browse/GEODE-5485).
> > >
> > > We could change FastLogger to override every Logger method instead of
> > > isDebugEnabled and isTraceEnabled. Then we could remove every single
> > guard
> > > clause from the Geode log statements.
> > >
> > > If Log4J2 changed their implementation of debug and trace to be
> > comparable
> > > with hitting a volatile for disabled statements then we could delete
> > > FastLogger and every log statement guard clause.
> > >
> > > On Mon, Sep 10, 2018 at 10:12 AM, Kirk Lund  wrote:
> > >
> > >> The reason we use these guards is that the Log4j2 implementation is
> more
> > >> expensive than hitting a volatile. Please don't change anything unless
> > you
> > >> start writing lots of JMH benchmarks! The existing code went through
> > lots
> > >> of iterations of perf testing that isn't part of Geode.
> > >>
> > >> Every Log4j2 Logger used by Geode classes are wrapped inside a
> > FastLogger
> > >> which uses a single volatile for those "isDebugEnabled" checks. If you
> > >> remove this, you will find that the performance of Geode will thank
> > even at
> > >> higher log levels such as INFO. Geode is currently optimized for INFO
> > level
> > >> logging. Without FastLogger, every log statement code path goes down
> > into a
> > >> hot mess of checking filters and performing file checking against
> > >> log4j2.xml (based on a mod so that it only occurs every several log
> > >> statements) to see if it has changed.
> > >>
> > >> Despite this, Log4J2 Core still out performs Logback for "disabled log
> > >> statements" and by this I mean the huge # of debug/trace level
> > statements
> > >> in Geode when running at INFO level.
> > >>
> > >> Unwrapping those "isDebugEnabled" checks will eliminate the
> optimization
> > >> provided by FastLogger. Without the guard clauses, FastLogger doesn't
> > >> improve anything because the Log4j2 code skips the "isDebugEnabled"
> > checks
> > >> and goes into filter checking which also checks the log level but
> after
> > >> much more work. My recommendation is to work with Log4j2 project to
> fix
> > >> this performance problem when using log statements without the
> > FastLogger
> > >> optimizations. For example, Log4j1 used a dedicated async thread for
> the
> > >> checking of the config file but for some reason they didn't go this
> > route
> > >> in Log4J2.
> > >>
> > >> PS: My Log4J2 code knowledge is a couple of years old so please feel
> > free
> > >> to dig into their code to see if the above issues were fixed.
> > >>
> > >> On Mon, Sep 10, 2018 at 9:35 AM, Galen O'Sullivan <
> > gosulli...@pivotal.io>
> > >> wrote:
> > 

Re: [DISCUSS] Wrapping log calls in Conditionals like isDebugEnabled, isTraceEnabled, etc.

[John Blum]

I think any arguments about what optimizations the (JIT enabled) compiler
(HotSpot or otherwise) will perform at runtime is questionable at best.
HotSpot can "inline" certain [frequent/hot] code paths both at
compile/runtime thereby reducing the number of method invocations (which
also depends on many factors, not the least of which is method size).

Here is a good SO post on the subject of Lambdas...

https://stackoverflow.com/questions/40860859/what-is-the-maximum-number-of-lambdas-used-in-one-java-class

I find it somewhat surprising that Geode (in any part of its codebase) hit
the limit on the number of Lambdas (methods).  That suggests that class
themselves need to be refactored and better organized by concern.


On Tue, Sep 11, 2018 at 9:02 AM, Dale Emery  wrote:

> If there’s enough duplication in the lambdas, or in the code around the
> lambdas, extracting the duplication into methods would reduce the number of
> lambdas.
>
> > On Sep 10, 2018, at 11:03 AM, Kirk Lund  wrote:
> >
> > Alright I'm more up-to-date on this thread. Some things to consider:
> >
> > The lambdas look great, but we'd have to start splitting the Geode
> clasess.
> > They're so big right now, that if you change a bunch of log statements to
> > use lambdas you'll hit the max number of lambdas on many of our classes.
> We
> > hit the lambda limit on a dunit test and it didn't really take that many
> > lambdas to hit the limit (see
> > https://issues.apache.org/jira/browse/GEODE-5485).
> >
> > We could change FastLogger to override every Logger method instead of
> > isDebugEnabled and isTraceEnabled. Then we could remove every single
> guard
> > clause from the Geode log statements.
> >
> > If Log4J2 changed their implementation of debug and trace to be
> comparable
> > with hitting a volatile for disabled statements then we could delete
> > FastLogger and every log statement guard clause.
> >
> > On Mon, Sep 10, 2018 at 10:12 AM, Kirk Lund  wrote:
> >
> >> The reason we use these guards is that the Log4j2 implementation is more
> >> expensive than hitting a volatile. Please don't change anything unless
> you
> >> start writing lots of JMH benchmarks! The existing code went through
> lots
> >> of iterations of perf testing that isn't part of Geode.
> >>
> >> Every Log4j2 Logger used by Geode classes are wrapped inside a
> FastLogger
> >> which uses a single volatile for those "isDebugEnabled" checks. If you
> >> remove this, you will find that the performance of Geode will thank
> even at
> >> higher log levels such as INFO. Geode is currently optimized for INFO
> level
> >> logging. Without FastLogger, every log statement code path goes down
> into a
> >> hot mess of checking filters and performing file checking against
> >> log4j2.xml (based on a mod so that it only occurs every several log
> >> statements) to see if it has changed.
> >>
> >> Despite this, Log4J2 Core still out performs Logback for "disabled log
> >> statements" and by this I mean the huge # of debug/trace level
> statements
> >> in Geode when running at INFO level.
> >>
> >> Unwrapping those "isDebugEnabled" checks will eliminate the optimization
> >> provided by FastLogger. Without the guard clauses, FastLogger doesn't
> >> improve anything because the Log4j2 code skips the "isDebugEnabled"
> checks
> >> and goes into filter checking which also checks the log level but after
> >> much more work. My recommendation is to work with Log4j2 project to fix
> >> this performance problem when using log statements without the
> FastLogger
> >> optimizations. For example, Log4j1 used a dedicated async thread for the
> >> checking of the config file but for some reason they didn't go this
> route
> >> in Log4J2.
> >>
> >> PS: My Log4J2 code knowledge is a couple of years old so please feel
> free
> >> to dig into their code to see if the above issues were fixed.
> >>
> >> On Mon, Sep 10, 2018 at 9:35 AM, Galen O'Sullivan <
> gosulli...@pivotal.io>
> >> wrote:
> >>
> >>> I think that logging in hot paths/loops is probably something that
> should
> >>> be avoided. And if it is necessary, I suspect that the JIT will
> >>> short-circuit that call since debug levels don't generally change.
> >>>
> >>> There probably are a few hot paths that need to optimize logging, but
> >>> that's not the majority.
> >>>
> >>> Can we agree to avoid this pattern in new code, since it adversely
> affects
> >>> readability?
> >>>
> >>> Galen
> >>>
> >>>
> >>> On Fri, Sep 7, 2018 at 1:46 PM, Nicholas Vallely 
> >>> wrote:
> >>>
>  I was just randomly looking into this a couple of days ago as a
> tangent
> >>> to
>  'observability' and came across this Stack Overflow:
>  https://stackoverflow.com/questions/6504407/is-there-a-need-
> >>> to-do-a-iflog-
>  isdebugenabled-check
>  where the first answer is the most succinct in my opinion.
> 
>  In the geode code that I searched, we are not consistent with our use
> of
>  the if(statement) wrapper for debug, though 

Re: [DISCUSS] Wrapping log calls in Conditionals like isDebugEnabled, isTraceEnabled, etc.

[Dale Emery]

If there’s enough duplication in the lambdas, or in the code around the 
lambdas, extracting the duplication into methods would reduce the number of 
lambdas.

> On Sep 10, 2018, at 11:03 AM, Kirk Lund  wrote:
> 
> Alright I'm more up-to-date on this thread. Some things to consider:
> 
> The lambdas look great, but we'd have to start splitting the Geode clasess.
> They're so big right now, that if you change a bunch of log statements to
> use lambdas you'll hit the max number of lambdas on many of our classes. We
> hit the lambda limit on a dunit test and it didn't really take that many
> lambdas to hit the limit (see
> https://issues.apache.org/jira/browse/GEODE-5485).
> 
> We could change FastLogger to override every Logger method instead of
> isDebugEnabled and isTraceEnabled. Then we could remove every single guard
> clause from the Geode log statements.
> 
> If Log4J2 changed their implementation of debug and trace to be comparable
> with hitting a volatile for disabled statements then we could delete
> FastLogger and every log statement guard clause.
> 
> On Mon, Sep 10, 2018 at 10:12 AM, Kirk Lund  wrote:
> 
>> The reason we use these guards is that the Log4j2 implementation is more
>> expensive than hitting a volatile. Please don't change anything unless you
>> start writing lots of JMH benchmarks! The existing code went through lots
>> of iterations of perf testing that isn't part of Geode.
>> 
>> Every Log4j2 Logger used by Geode classes are wrapped inside a FastLogger
>> which uses a single volatile for those "isDebugEnabled" checks. If you
>> remove this, you will find that the performance of Geode will thank even at
>> higher log levels such as INFO. Geode is currently optimized for INFO level
>> logging. Without FastLogger, every log statement code path goes down into a
>> hot mess of checking filters and performing file checking against
>> log4j2.xml (based on a mod so that it only occurs every several log
>> statements) to see if it has changed.
>> 
>> Despite this, Log4J2 Core still out performs Logback for "disabled log
>> statements" and by this I mean the huge # of debug/trace level statements
>> in Geode when running at INFO level.
>> 
>> Unwrapping those "isDebugEnabled" checks will eliminate the optimization
>> provided by FastLogger. Without the guard clauses, FastLogger doesn't
>> improve anything because the Log4j2 code skips the "isDebugEnabled" checks
>> and goes into filter checking which also checks the log level but after
>> much more work. My recommendation is to work with Log4j2 project to fix
>> this performance problem when using log statements without the FastLogger
>> optimizations. For example, Log4j1 used a dedicated async thread for the
>> checking of the config file but for some reason they didn't go this route
>> in Log4J2.
>> 
>> PS: My Log4J2 code knowledge is a couple of years old so please feel free
>> to dig into their code to see if the above issues were fixed.
>> 
>> On Mon, Sep 10, 2018 at 9:35 AM, Galen O'Sullivan 
>> wrote:
>> 
>>> I think that logging in hot paths/loops is probably something that should
>>> be avoided. And if it is necessary, I suspect that the JIT will
>>> short-circuit that call since debug levels don't generally change.
>>> 
>>> There probably are a few hot paths that need to optimize logging, but
>>> that's not the majority.
>>> 
>>> Can we agree to avoid this pattern in new code, since it adversely affects
>>> readability?
>>> 
>>> Galen
>>> 
>>> 
>>> On Fri, Sep 7, 2018 at 1:46 PM, Nicholas Vallely 
>>> wrote:
>>> 
 I was just randomly looking into this a couple of days ago as a tangent
>>> to
 'observability' and came across this Stack Overflow:
 https://stackoverflow.com/questions/6504407/is-there-a-need-
>>> to-do-a-iflog-
 isdebugenabled-check
 where the first answer is the most succinct in my opinion.
 
 In the geode code that I searched, we are not consistent with our use of
 the if(statement) wrapper for debug, though most do have the wrapper.
 
 Nick
 
 On Fri, Sep 7, 2018 at 11:35 AM Jacob Barrett 
>>> wrote:
 
> Checking with logger.isDebugEnabled() it still a good practice for hot
> paths to avoid the construction of intermediate object arrays to hold
>>> the
> var-args. Some logging libraries have fixed argument optimizations for
> var-args up to a specific limit. In very hot paths it is nice to
> check logger.isDebugEnabled() once into a temp boolean value and then
 check
> that in all the subsequent debug logging statements in the hot path.
> 
> -Jake
> 
> 
> On Fri, Sep 7, 2018 at 11:18 AM Dan Smith  wrote:
> 
>> I think this pattern is a holdover from using log4j 1. In that
>>> version,
> you
>> added an if check to avoid unnecessary string concatenation if debug
 was
>> enabled.
>> 
>> 
>>   1. if (logger.isDebugEnabled()) {
>>   2. logger.debug("Logging in user " + 

Speakers needed for Apache DC Roadshow

[Rich Bowen]

We need your help to make the Apache Washington DC Roadshow on Dec 4th a 
success.


What do we need most? Speakers!

We're bringing a unique DC flavor to this event by mixing Open Source 
Software with talks about Apache projects as well as OSS CyberSecurity, 
OSS in Government and and OSS Career advice.


Please take a look at: http://www.apachecon.com/usroadshow18/

(Note: You are receiving this message because you are subscribed to one 
or more mailing lists at The Apache Software Foundation.)


Rich, for the ApacheCon Planners

--
rbo...@apache.org
http://apachecon.com
@ApacheCon