The POILogger is actually a reasonably good implementation.
default void log(int level, Object... objs) {
if (!check(level)) return;
https://github.com/apache/poi/blob/trunk/src/java/org/apache/poi/util/POILogger.java
This is basically what slf4j and log4j do too. The objs will not be appended
into a string if the log level is not required.
I would still favour modifying POILogger to use slf4j under the hood but this
saves us having to rewrite all the logging code. If there is a better API than
the one above, we can add extra methods to POILogger when needed and switch
some logging to use the new methods over time.
I would favour slf4j because users can choose which logging implemenatation
they prefer as opposed to us mandating a specific framework life log4j.
If noone objects, I can make the slf4j change to POILogger before the 5.0.0
release.
On Tuesday 8 December 2020, 20:43:35 GMT, Dominik Stadler
<dominik.stad...@gmx.at> wrote:
Hi,
I also see the custom logging functionality unnecessary and outdated,
replacing it with one of the popular log-frameworks would be nice.
However I see POI mostly as a tool-library which usually should only log
very few things, mostly only warnings in some cases when it is being used
incorrectly in some way. We then be able to enable debug logs with some
configuration change to investigate things if necessary.
Choosing the "right" logging framework is hard for me, unfortunately there
are many choices. When building some application, I usually end up with
many different ones dragged in via dependencies. So a solution which uses
one of the small wrappers which can plug into most of the common frameworks
would be nice. I think slf4j can do this as well as commons-logging. Log4J
is the one closes to being the de-facto standard, but would be a bit harder
to integrate into larger applications, although I don't have much
experience with doing this, so it might work well. I would at least try it
before making a decision.
On performance/allocations: we did perform a fair amount of testing of
Apache POI over the years and logging was never a concern as far as I know,
probably because there are very few logging statements in the code anyway.
So you will likely not see much of it compared to other allocations. But
there might be cases where it becomes more prominent.
Thanks... Dominik.
On Sun, Dec 6, 2020 at 12:26 AM Dave Fisher <w...@apache.org> wrote:
> FYI -
>
> POLogger was part of the initial commit of POI to the Apache Jakarta
> project on Jan. 31, 2002.
>
> https://svn.apache.org/viewvc?view=revision&revision=352063 <
> https://svn.apache.org/viewvc?view=revision&revision=352063>
>
> History from svn can be found here:
>
> https://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/util/POILogFactory.java?view=log
> <
> https://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/util/POILogFactory.java?view=log
> >
>
> https://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/util/POILogger.java?view=log
> <
> https://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/util/POILogger.java?view=log
> >
>
> There were many bug fixes, etc.
>
> Regards,
> Dave
>
> > On Dec 5, 2020, at 2:46 PM, Bill Taylor <watay...@alum.mit.edu> wrote:
> >
> > Good idea. I've been through the slf 4 j transition in other places. It
> > is PAINFUL and nobody wants to do it, but the resulting system is a lot
> > better.
> >
> > On Sat, Dec 5, 2020 at 5:36 PM Marius Volkhart <mar...@volkhart.com>
> wrote:
> >
> >> Hello,
> >>
> >> POI currently uses a custom facility for logging: the POILogger
> interface.
> >> This serves as a level of indirection to allow consumers to plug in a
> >> logging implementation of their own. Best as I can tell, this facility
> was
> >> added nearly 10 years ago, and the logging landscape has changed a lot
> in
> >> that time. I'd like to revisit this facility.
> >>
> >> The impact of temporary objects due to logging has been of increasing
> focus
> >> in the last few years. Projects like Apache Log4J have gone to great
> >> lengths to reduce the number of temporary objects created for logging
> >> purposes, and run garbage-free in some configurations. At this time,
> >> POILogger is comparatively garbage heavy. Every call to
> POILogger.log(int,
> >> Object...) requires at least an Object[] allocation, even if the message
> >> won't be logged. In many cases, string concatenation is done before
> calling
> >> log(int, Object...), resulting in more allocations. The logging
> frameworks
> >> like Log4J have solved this by adding overloads that take a varying
> number
> >> of arguments.
> >>
> >> Once inside of log(int, Object...), if it is determined that the message
> >> should be logged, the common case of a single Object array is not
> optimized
> >> for. Furthermore, the log forging escape regex is compiled every time.
> >>
> >> Despite providing an indirection of it's own, POI still has a hard
> >> dependency on Apache Commons Logging, and ships with a POILogger
> >> implementation that defers to Commons Logging.
> >>
> >> The performance topics could all be addressed without much difficulty.
> The
> >> question I pose is, should they be? Other Apache projects exist to
> focus on
> >> the problem of how to do logging well. I propose that POI uses one of
> the
> >> logging frameworks directly, rather than having the POILogger
> abstraction.
> >> 10 years ago, logging frameworks didn't split their API and backend the
> way
> >> that Log4J and SLF4J/Logback do today, so the need for an indirection
> was
> >> greater.
> >>
> >> Commons Logging is already a dependency, so it may seem a logical
> choice is
> >> to use it directly. However, Commons Logging has a very limited API,
> which
> >> would make POI authors write more logging code than they do now. It also
> >> suffers from high garbage.
> >>
> >> Instead, I propose that Log4J is the more logical choice. It's a popular
> >> logging framework, under the Apache umbrella, and is a leader when it
> comes
> >> to considering performance while providing rich logging APIs. Using the
> >> Log4J API lets application authors decide if they want to use Logback,
> >> Log4J Core, or some other logging backend, and gives them a lot of
> >> flexibility in how logging is done.
> >>
> >> Of course, removing POILogger is an API change. However, it is marked as
> >> being @Internal, and previous discussion
> >> <https://bz.apache.org/bugzilla/show_bug.cgi?id=63047> suggests there
> are
> >> unlikely to be many users of this API. This feels like a change that
> could
> >> happen after the 5.0 release.
> >>
> >> I would like to complete this work, and would also write new
> documentation
> >> to explain how POI submits log events to the Log4J API.
> >>
> >> Proposed changes:
> >> - Remove Commons Logging dependency
> >> - Add Log4J API dependency
> >> - Remove POILogger API
> >> - Remove POILogFactory API
> >> - Modify all log sites to use Log4J API
> >> - Write new documentation for how POI submits events
> >>
> >> --
> >> Cheers,
> >> Marius Volkhart
> >>
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@poi.apache.org
For additional commands, e-mail: dev-h...@poi.apache.org
