You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@harmony.apache.org by "Jesse Wilson (JIRA)" <ji...@apache.org> on 2009/10/26 21:33:02 UTC
[jira] Created: (HARMONY-6362) Logging performance improvements
Logging performance improvements
--------------------------------
Key: HARMONY-6362
URL: https://issues.apache.org/jira/browse/HARMONY-6362
Project: Harmony
Issue Type: Improvement
Components: Classlib
Environment: SVN Revision: 829932
Reporter: Jesse Wilson
Priority: Minor
The logging module performs more synchronization and object allocation than necessary. In particular, getHandlers() always needs synchronization and creates an array. By adopting util.concurrent features (CopyOnWriteArrayList) and coding to the common case (when there are zero handlers) I saw a 2.5x improvement in throughput for calls to logger.log(Level, String). In a benchmark that measures calls to "logger.info", call time improved from 385000ns to 154000ns per message.
Here's an overview of the patch:
- Reduce the use of synchronization by adopting a CopyOnWriteArrayList for Handlers
- Reduce object creation by reusing a 0-length handlers array in getHandlers()
- Load handlers on logger creation rather than on receipt of the first message. This also fixes a behavioural inconsistency with the RI. I've got a test case that demonstrates this.
- Cleanup synchronization by moving methods with "synchronized (LogManager.getManager())" blocks to the LogManager class
I've got the patch ready to submit upstream. I'll submit it when my commit privileges are granted, unless anyone raises objections here.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
Re: [classlib][modularity] Logging performance improvements (HARMONY-6362)
Posted by Tim Ellison <t....@gmail.com>.
On 10/Nov/2009 19:38, Jesse Wilson wrote:
> On Tue, Nov 10, 2009 at 1:47 AM, Mark Hindess
> <ma...@googlemail.com>wrote:
>
>> Since they aren't active on the list I'm not sure how much we should let
>> this influence our decision. It does suggest that we should be cautious
>> about making assumptions about what downstream users might like to do.
>> I am reluctant to limit subsetting choices for (potential) downstream
>> users without good reason.
>
> Well I guess the burden is on myself to provide a good reason! The
> java.util.concurrent package is excellent; it is Java's best API. By
> permitting its use by other modules, we will see increased correctness and
> performance...
>
> Consider java.net.NegativeCache. This class uses a global lock to interact
> with a LinkedHashMap concurrently. Upgrading to a more modern datastructure
> like a ConcurrentHashMap can improve application throughput. Similarly for
> the half dozen other caches in our code.
>
> Or perhaps InetAddress.isReachableByMultiThread(), which currently ignores
> InterruptedExceptions while its background thread is working.
>
> And there's SystemProcess, whose create() method contains 4 synchronized
> blocks. Perhaps this could benefit from java.util.concurrent? As it's
> written currently, an OutOfMemoryError while allocating one of the three
> buffers will cause the create() call to hang indefinitely.
>
> Writing code in 2009 without java.util.concurrent is like writing code with
> your hand tied behind your back.
You *are* allowed to use your feet too though.
Tim
Re: [classlib][modularity] Logging performance improvements
(HARMONY-6362)
Posted by Jesse Wilson <je...@google.com>.
On Tue, Nov 10, 2009 at 1:47 AM, Mark Hindess
<ma...@googlemail.com>wrote:
> Since they aren't active on the list I'm not sure how much we should let
> this influence our decision. It does suggest that we should be cautious
> about making assumptions about what downstream users might like to do.
> I am reluctant to limit subsetting choices for (potential) downstream
> users without good reason.
>
Well I guess the burden is on myself to provide a good reason! The
java.util.concurrent package is excellent; it is Java's best API. By
permitting its use by other modules, we will see increased correctness and
performance...
Consider java.net.NegativeCache. This class uses a global lock to interact
with a LinkedHashMap concurrently. Upgrading to a more modern datastructure
like a ConcurrentHashMap can improve application throughput. Similarly for
the half dozen other caches in our code.
Or perhaps InetAddress.isReachableByMultiThread(), which currently ignores
InterruptedExceptions while its background thread is working.
And there's SystemProcess, whose create() method contains 4 synchronized
blocks. Perhaps this could benefit from java.util.concurrent? As it's
written currently, an OutOfMemoryError while allocating one of the three
buffers will cause the create() call to hang indefinitely.
Writing code in 2009 without java.util.concurrent is like writing code with
your hand tied behind your back.
Re: [classlib][modularity] Logging performance improvements (HARMONY-6362)
Posted by Mark Hindess <ma...@googlemail.com>.
In message <a4...@mail.gmail.com>,
Jesse Wilson writes:
>
> How do you feel about saying the core is "LUUUUUUUNI" ?
> - lang
> - util
> - util.concurrent.*
> - util.jar
> - util.regex
> - util.logging
> - util.prefs
> - util.zip
> - net
> - io
> This feels like a good fit to me, particularly since the RI exposes
> API interdependencies between these packages.
Looking at the dlls/jars in this list:
http://www.webos-internals.org/wiki/WebOS_Doctor_version_1.1.0
it would seem that the Palm Pre uses harmony but does not include
concurrent or prefs.
Since they aren't active on the list I'm not sure how much we should let
this influence our decision. It does suggest that we should be cautious
about making assumptions about what downstream users might like to do.
I am reluctant to limit subsetting choices for (potential) downstream
users without good reason.
Regards,
-Mark.
Re: [classlib][modularity] Logging performance improvements (HARMONY-6362)
Posted by Tim Ellison <t....@gmail.com>.
On 10/Nov/2009 03:52, Nathan Beyer wrote:
> On Mon, Nov 9, 2009 at 4:31 PM, Jesse Wilson <je...@google.com> wrote:
>> On Mon, Nov 9, 2009 at 4:14 AM, Tim Ellison <t....@gmail.com> wrote:
>>
>>> I didn't mean to ask "Why is a CopyOnWriteArrayList useful?", rather
>>> what did it contribute to your logging improvements, e.g. correctness
>>> that would require tons of duplicate coding otherwise, or performance
>>> numbers, etc. (however, see below)
>>>
>> Using CopyOnWriteArrayList permitted me to strip off "synchronized" from
>> getHandlers(). This makes it possible for multiple threads to log messages
>> concurrently without contention.
>>
>>
>> The ability to consume modules from harmony without having to deal with
>>> inter-module spaghetti code is worth preserving.
>>>
>> Okay, I'll bite. Why is it worth preserving? We agree that it's useful for
>> projects to take some modules from Harmony and some from external sources.
>> Android gets most of its modules from Harmony, but has its own regex module.
>> But Android still *has* a regex module.
>>
>> Is anyone reusing modules from Harmony on a system that won't have a
>> java.util.concurrent package? I think it should be fair game for modules to
>> depend upon the published APIs of other modules in their implementation
>> details.
>
> If it's out there, they certainly haven't popped up to say anything.
>
> Over the past few years I've gotten less and less interested in
> asserting the loose coupling of the modules. At this point, I'd rather
> go for reducing the module count and go for some sort of non-milestone
> release.
I already mooted the point earlier this year,
http://markmail.org/thread/4kf7zlz2qujuemxw
There was general approval so if you want to take a crack at it, go ahead.
Regards,
Tim
> I certainly think there are separations that should be
> maintained, but I think there are a few that could be rolled together
> without any real concern.
>
> - Pull in all of the java.util.* into LUNI
> - Pull annotation into LUNI
> - Consider pulling lang-management into LUNI (this one's tough because
> of jmx dependencies)
> - Pull AWT and Swing together
> - Pull nio-char into nio
>
> Honestly, at this point, I think the biggest strength of Harmony's
> Class Library is the separation of UI and non-UI stuff. A JVM +
> headless classlibrary is very attractive.
>
>>
>> I'm prepared to accept that concurrent becomes part of the fundamental
>>> core classes that are used to implement the remainder of the class
>>> libraries. I'd be less happy about seeing lots of references to, say,
>>> logging scattered throughout the other modules. We should remain modest
>>> in our dependencies.
>>>
>> In Android the luni module depends on our logging module! From our
>> BufferedOutputStream<http://android.git.kernel.org/?p=platform/dalvik.git;a=blob;f=libcore/luni/src/main/java/java/io/BufferedOutputStream.java;h=835d13f153b6d2baff48621a596aa531bf42fffb;hb=master>class,
>>
>> 68 <#l68> public BufferedOutputStream(OutputStream out) {
>> 69 <#l69> super(out);
>> 70 <#l70> buf = new byte[8192];
>> 71 <#l71>
>> 72 <#l72> // BEGIN android-added
>> 73 <#l73> /*
>> 74 <#l74> * For Android, we want to discourage the use of this
>> constructor (with
>> 75 <#l75> * its arguably too-large default), so we note its
>> use in the log. We
>> 76 <#l76> * don't disable it, nor do we alter the default,
>> however, because we
>> 77 <#l77> * still aim to behave compatibly, and the default
>> value, though not
>> 78 <#l78> * documented, is established by convention.
>> 79 <#l79> */
>> 80 <#l80> Logger.global.info(
>> 81 <#l81> "Default buffer size used in BufferedOutputStream " +
>> 82 <#l82> "constructor. It would be " +
>> 83 <#l83> "better to be explicit if an 8k buffer is required.");
>> 84 <#l84> // END android-added
>> 85 <#l85> }
>>
>>
>> Java.util.logging provided a suitable vehicle for emitting a warning. We
>> haven't seen any problems caused by the coupling between luni and logging
>> modules. Now I'm not suggesting that we rush in to add coupling between
>> Harmony's modules, but I fail to see how the coupling is harmful. I also
>> think that the duplication in the resource bundling code is far worse than
>> the coupling it prevents.
>>
>
Re: [classlib][modularity] Logging performance improvements
(HARMONY-6362)
Posted by Nathan Beyer <nd...@apache.org>.
On Mon, Nov 9, 2009 at 4:31 PM, Jesse Wilson <je...@google.com> wrote:
> On Mon, Nov 9, 2009 at 4:14 AM, Tim Ellison <t....@gmail.com> wrote:
>
>> I didn't mean to ask "Why is a CopyOnWriteArrayList useful?", rather
>> what did it contribute to your logging improvements, e.g. correctness
>> that would require tons of duplicate coding otherwise, or performance
>> numbers, etc. (however, see below)
>>
>
> Using CopyOnWriteArrayList permitted me to strip off "synchronized" from
> getHandlers(). This makes it possible for multiple threads to log messages
> concurrently without contention.
>
>
> The ability to consume modules from harmony without having to deal with
>> inter-module spaghetti code is worth preserving.
>>
>
> Okay, I'll bite. Why is it worth preserving? We agree that it's useful for
> projects to take some modules from Harmony and some from external sources.
> Android gets most of its modules from Harmony, but has its own regex module.
> But Android still *has* a regex module.
>
> Is anyone reusing modules from Harmony on a system that won't have a
> java.util.concurrent package? I think it should be fair game for modules to
> depend upon the published APIs of other modules in their implementation
> details.
If it's out there, they certainly haven't popped up to say anything.
Over the past few years I've gotten less and less interested in
asserting the loose coupling of the modules. At this point, I'd rather
go for reducing the module count and go for some sort of non-milestone
release. I certainly think there are separations that should be
maintained, but I think there are a few that could be rolled together
without any real concern.
- Pull in all of the java.util.* into LUNI
- Pull annotation into LUNI
- Consider pulling lang-management into LUNI (this one's tough because
of jmx dependencies)
- Pull AWT and Swing together
- Pull nio-char into nio
Honestly, at this point, I think the biggest strength of Harmony's
Class Library is the separation of UI and non-UI stuff. A JVM +
headless classlibrary is very attractive.
>
>
> I'm prepared to accept that concurrent becomes part of the fundamental
>> core classes that are used to implement the remainder of the class
>> libraries. I'd be less happy about seeing lots of references to, say,
>> logging scattered throughout the other modules. We should remain modest
>> in our dependencies.
>>
>
> In Android the luni module depends on our logging module! From our
> BufferedOutputStream<http://android.git.kernel.org/?p=platform/dalvik.git;a=blob;f=libcore/luni/src/main/java/java/io/BufferedOutputStream.java;h=835d13f153b6d2baff48621a596aa531bf42fffb;hb=master>class,
>
> 68 <#l68> public BufferedOutputStream(OutputStream out) {
> 69 <#l69> super(out);
> 70 <#l70> buf = new byte[8192];
> 71 <#l71>
> 72 <#l72> // BEGIN android-added
> 73 <#l73> /*
> 74 <#l74> * For Android, we want to discourage the use of this
> constructor (with
> 75 <#l75> * its arguably too-large default), so we note its
> use in the log. We
> 76 <#l76> * don't disable it, nor do we alter the default,
> however, because we
> 77 <#l77> * still aim to behave compatibly, and the default
> value, though not
> 78 <#l78> * documented, is established by convention.
> 79 <#l79> */
> 80 <#l80> Logger.global.info(
> 81 <#l81> "Default buffer size used in BufferedOutputStream " +
> 82 <#l82> "constructor. It would be " +
> 83 <#l83> "better to be explicit if an 8k buffer is required.");
> 84 <#l84> // END android-added
> 85 <#l85> }
>
>
> Java.util.logging provided a suitable vehicle for emitting a warning. We
> haven't seen any problems caused by the coupling between luni and logging
> modules. Now I'm not suggesting that we rush in to add coupling between
> Harmony's modules, but I fail to see how the coupling is harmful. I also
> think that the duplication in the resource bundling code is far worse than
> the coupling it prevents.
>
Re: [classlib][modularity] Logging performance improvements (HARMONY-6362)
Posted by Mark Hindess <ma...@googlemail.com>.
In message <96...@mail.gmail.com>,
enh writes:
>
> [btw, i followed the link, but didn't really understand what i was
> looking at or how it demonstrated the case.]
Sorry, I probably assumed too much history[0]. That link pointed to a
list of files that changed between releases of WebOS (the OS on the Palm
Pre). If you examine the list of files you will find references to some
Harmony artifacts but not concurrent.jar. One could assume that it just
didn't change but I doubt that is the case since jars tend to change
with any rebuild due to containing timestamps of included files.
Regards,
Mark.
[0] http://markmail.org/message/5gyqg3y6mswlp6gp
Re: [classlib][modularity] Logging performance improvements
(HARMONY-6362)
Posted by enh <en...@google.com>.
On Thu, Nov 12, 2009 at 02:44, Tim Ellison <t....@gmail.com> wrote:
> On 09/Nov/2009 22:31, Jesse Wilson wrote:
>> On Mon, Nov 9, 2009 at 4:14 AM, Tim Ellison <t....@gmail.com> wrote:
>>
>>> I didn't mean to ask "Why is a CopyOnWriteArrayList useful?", rather
>>> what did it contribute to your logging improvements, e.g. correctness
>>> that would require tons of duplicate coding otherwise, or performance
>>> numbers, etc. (however, see below)
>>
>> Using CopyOnWriteArrayList permitted me to strip off "synchronized" from
>> getHandlers(). This makes it possible for multiple threads to log messages
>> concurrently without contention.
>>
>>> The ability to consume modules from harmony without having to deal with
>>> inter-module spaghetti code is worth preserving.
>>
>> Okay, I'll bite. Why is it worth preserving? We agree that it's useful for
>> projects to take some modules from Harmony and some from external sources.
>> Android gets most of its modules from Harmony, but has its own regex module.
>> But Android still *has* a regex module.
>
> And I'd argue that the reason groups like Android /can/ pick up
> alternative implementations is because we minimize the coupling between
> the modules.
>
> Maybe we need to make a finer distinction,
>
> (a) non-API dependencies: so to take your example, if (say) LUNI had
> links into the internal workings of Harmony's regex implementation there
> would have been more places for you to hack the LUNI code to make it
> work with your implementation of regex. Minimizing the
> non-(standard)-API dependencies is a good thing.
i don't think anyone's arguing otherwise.
> (b) API dependencies: some are placed upon us by the spec, and some are
> quite bizzare or unfortunate. So I the reason I need to have an Applet
> implementation in order to compile our Beans implementation is because
> there is a standard API dependency between them. But without such
> requirements it would be madness to have such gratuitous dependencies.
again, i don't think anyone's arguing otherwise.
i forget who it was made a distinction between headless and
non-headless classes, but i think that's a good one. "java" versus
"javax/org/com" is a pretty good one too.
> The knack is deciding which are gratuitous, and which are reasonable.
> I'm happy to say that concurrent is common enough and a useful utility
> that we can say 'go at it' to implement the other modules in terms of
> concurrency APIs -- but if some bright spark wants to use the event
> mechanism from Swing then I'd object.
>
> (Not a fictitious example, there are plenty of examples of notionally
> headless code that drag in this dependency.)
i wouldn't have believed it if i hadn't seen it with my own eyes, but
there was an Android bug asking us to include the swing model classes
because they had some non-gui code that was using [iirc]
DefaultTreeModel.
(obviously, we agree that's a bad idea, and don't want to encourage it.)
>> Is anyone reusing modules from Harmony on a system that won't have a
>> java.util.concurrent package?
>
> As Mark points out, yes there are apparently; but if they are not
> prepared to come here and defend the argument then we get to make the
> decisions for them <g>.
we can also use "argument by authority" and point out that the back
cover of Effective Java says "...the language and its most fundamental
libraries: java.lang, java.util, and, to a lesser extent,
java.util.concurrent and java.io" ;-)
[btw, i followed the link, but didn't really understand what i was
looking at or how it demonstrated the case.]
>> I think it should be fair game for modules to depend upon the
>> published APIs of other modules in their implementation details.
>
> Within reason.
maybe a useful rule of thumb is "if we wrote something here from
scratch, would it be better or worse than reusing existing public
classes?".
in the DefaultTreeModel case, special-purpose code would be a more
intention-revealing model of what they were really doing, and would be
small and hard to get wrong.
in the case of some internal cache, there's almost nothing to gain
from rolling our own concurrent data structures and any half-decent
implementation is large and hard to do well.
--elliott
>>> I'm prepared to accept that concurrent becomes part of the fundamental
>>> core classes that are used to implement the remainder of the class
>>> libraries. I'd be less happy about seeing lots of references to, say,
>>> logging scattered throughout the other modules. We should remain modest
>>> in our dependencies.
>>
>> In Android the luni module depends on our logging module! From our
>> BufferedOutputStream<http://android.git.kernel.org/?p=platform/dalvik.git;a=blob;f=libcore/luni/src/main/java/java/io/BufferedOutputStream.java;h=835d13f153b6d2baff48621a596aa531bf42fffb;hb=master>class,
>>
>> 68 <#l68> public BufferedOutputStream(OutputStream out) {
>> 69 <#l69> super(out);
>> 70 <#l70> buf = new byte[8192];
>> 71 <#l71>
>> 72 <#l72> // BEGIN android-added
>> 73 <#l73> /*
>> 74 <#l74> * For Android, we want to discourage the use of this
>> constructor (with
>> 75 <#l75> * its arguably too-large default), so we note its
>> use in the log. We
>> 76 <#l76> * don't disable it, nor do we alter the default,
>> however, because we
>> 77 <#l77> * still aim to behave compatibly, and the default
>> value, though not
>> 78 <#l78> * documented, is established by convention.
>> 79 <#l79> */
>> 80 <#l80> Logger.global.info(
>> 81 <#l81> "Default buffer size used in BufferedOutputStream " +
>> 82 <#l82> "constructor. It would be " +
>> 83 <#l83> "better to be explicit if an 8k buffer is required.");
>> 84 <#l84> // END android-added
>> 85 <#l85> }
>>
>>
>> Java.util.logging provided a suitable vehicle for emitting a warning. We
>> haven't seen any problems caused by the coupling between luni and logging
>> modules. Now I'm not suggesting that we rush in to add coupling between
>> Harmony's modules, but I fail to see how the coupling is harmful. I also
>> think that the duplication in the resource bundling code is far worse than
>> the coupling it prevents.
>
> I assume you mean the I18N resource handling? in which case, yes, I
> agree that is not such a bright idea. There are people who take modules
> (e.g. pack200) but not LUNI, I wouldn't mind if they had to copy the
> resource handling code themselves to be self-sufficient.
>
> Regards,
> Tim
>
>
>
--
Elliott Hughes - http://who/enh - http://jessies.org/~enh/
Re: [classlib][modularity] Logging performance improvements (HARMONY-6362)
Posted by Tim Ellison <t....@gmail.com>.
On 09/Nov/2009 22:31, Jesse Wilson wrote:
> On Mon, Nov 9, 2009 at 4:14 AM, Tim Ellison <t....@gmail.com> wrote:
>
>> I didn't mean to ask "Why is a CopyOnWriteArrayList useful?", rather
>> what did it contribute to your logging improvements, e.g. correctness
>> that would require tons of duplicate coding otherwise, or performance
>> numbers, etc. (however, see below)
>
> Using CopyOnWriteArrayList permitted me to strip off "synchronized" from
> getHandlers(). This makes it possible for multiple threads to log messages
> concurrently without contention.
>
>> The ability to consume modules from harmony without having to deal with
>> inter-module spaghetti code is worth preserving.
>
> Okay, I'll bite. Why is it worth preserving? We agree that it's useful for
> projects to take some modules from Harmony and some from external sources.
> Android gets most of its modules from Harmony, but has its own regex module.
> But Android still *has* a regex module.
And I'd argue that the reason groups like Android /can/ pick up
alternative implementations is because we minimize the coupling between
the modules.
Maybe we need to make a finer distinction,
(a) non-API dependencies: so to take your example, if (say) LUNI had
links into the internal workings of Harmony's regex implementation there
would have been more places for you to hack the LUNI code to make it
work with your implementation of regex. Minimizing the
non-(standard)-API dependencies is a good thing.
(b) API dependencies: some are placed upon us by the spec, and some are
quite bizzare or unfortunate. So I the reason I need to have an Applet
implementation in order to compile our Beans implementation is because
there is a standard API dependency between them. But without such
requirements it would be madness to have such gratuitous dependencies.
The knack is deciding which are gratuitous, and which are reasonable.
I'm happy to say that concurrent is common enough and a useful utility
that we can say 'go at it' to implement the other modules in terms of
concurrency APIs -- but if some bright spark wants to use the event
mechanism from Swing then I'd object.
(Not a fictitious example, there are plenty of examples of notionally
headless code that drag in this dependency.)
> Is anyone reusing modules from Harmony on a system that won't have a
> java.util.concurrent package?
As Mark points out, yes there are apparently; but if they are not
prepared to come here and defend the argument then we get to make the
decisions for them <g>.
> I think it should be fair game for modules to depend upon the
> published APIs of other modules in their implementation details.
Within reason.
>> I'm prepared to accept that concurrent becomes part of the fundamental
>> core classes that are used to implement the remainder of the class
>> libraries. I'd be less happy about seeing lots of references to, say,
>> logging scattered throughout the other modules. We should remain modest
>> in our dependencies.
>
> In Android the luni module depends on our logging module! From our
> BufferedOutputStream<http://android.git.kernel.org/?p=platform/dalvik.git;a=blob;f=libcore/luni/src/main/java/java/io/BufferedOutputStream.java;h=835d13f153b6d2baff48621a596aa531bf42fffb;hb=master>class,
>
> 68 <#l68> public BufferedOutputStream(OutputStream out) {
> 69 <#l69> super(out);
> 70 <#l70> buf = new byte[8192];
> 71 <#l71>
> 72 <#l72> // BEGIN android-added
> 73 <#l73> /*
> 74 <#l74> * For Android, we want to discourage the use of this
> constructor (with
> 75 <#l75> * its arguably too-large default), so we note its
> use in the log. We
> 76 <#l76> * don't disable it, nor do we alter the default,
> however, because we
> 77 <#l77> * still aim to behave compatibly, and the default
> value, though not
> 78 <#l78> * documented, is established by convention.
> 79 <#l79> */
> 80 <#l80> Logger.global.info(
> 81 <#l81> "Default buffer size used in BufferedOutputStream " +
> 82 <#l82> "constructor. It would be " +
> 83 <#l83> "better to be explicit if an 8k buffer is required.");
> 84 <#l84> // END android-added
> 85 <#l85> }
>
>
> Java.util.logging provided a suitable vehicle for emitting a warning. We
> haven't seen any problems caused by the coupling between luni and logging
> modules. Now I'm not suggesting that we rush in to add coupling between
> Harmony's modules, but I fail to see how the coupling is harmful. I also
> think that the duplication in the resource bundling code is far worse than
> the coupling it prevents.
I assume you mean the I18N resource handling? in which case, yes, I
agree that is not such a bright idea. There are people who take modules
(e.g. pack200) but not LUNI, I wouldn't mind if they had to copy the
resource handling code themselves to be self-sufficient.
Regards,
Tim
Re: [classlib][modularity] Logging performance improvements
(HARMONY-6362)
Posted by Jesse Wilson <je...@google.com>.
On Mon, Nov 9, 2009 at 4:14 AM, Tim Ellison <t....@gmail.com> wrote:
> I didn't mean to ask "Why is a CopyOnWriteArrayList useful?", rather
> what did it contribute to your logging improvements, e.g. correctness
> that would require tons of duplicate coding otherwise, or performance
> numbers, etc. (however, see below)
>
Using CopyOnWriteArrayList permitted me to strip off "synchronized" from
getHandlers(). This makes it possible for multiple threads to log messages
concurrently without contention.
The ability to consume modules from harmony without having to deal with
> inter-module spaghetti code is worth preserving.
>
Okay, I'll bite. Why is it worth preserving? We agree that it's useful for
projects to take some modules from Harmony and some from external sources.
Android gets most of its modules from Harmony, but has its own regex module.
But Android still *has* a regex module.
Is anyone reusing modules from Harmony on a system that won't have a
java.util.concurrent package? I think it should be fair game for modules to
depend upon the published APIs of other modules in their implementation
details.
I'm prepared to accept that concurrent becomes part of the fundamental
> core classes that are used to implement the remainder of the class
> libraries. I'd be less happy about seeing lots of references to, say,
> logging scattered throughout the other modules. We should remain modest
> in our dependencies.
>
In Android the luni module depends on our logging module! From our
BufferedOutputStream<http://android.git.kernel.org/?p=platform/dalvik.git;a=blob;f=libcore/luni/src/main/java/java/io/BufferedOutputStream.java;h=835d13f153b6d2baff48621a596aa531bf42fffb;hb=master>class,
68 <#l68> public BufferedOutputStream(OutputStream out) {
69 <#l69> super(out);
70 <#l70> buf = new byte[8192];
71 <#l71>
72 <#l72> // BEGIN android-added
73 <#l73> /*
74 <#l74> * For Android, we want to discourage the use of this
constructor (with
75 <#l75> * its arguably too-large default), so we note its
use in the log. We
76 <#l76> * don't disable it, nor do we alter the default,
however, because we
77 <#l77> * still aim to behave compatibly, and the default
value, though not
78 <#l78> * documented, is established by convention.
79 <#l79> */
80 <#l80> Logger.global.info(
81 <#l81> "Default buffer size used in BufferedOutputStream " +
82 <#l82> "constructor. It would be " +
83 <#l83> "better to be explicit if an 8k buffer is required.");
84 <#l84> // END android-added
85 <#l85> }
Java.util.logging provided a suitable vehicle for emitting a warning. We
haven't seen any problems caused by the coupling between luni and logging
modules. Now I'm not suggesting that we rush in to add coupling between
Harmony's modules, but I fail to see how the coupling is harmful. I also
think that the duplication in the resource bundling code is far worse than
the coupling it prevents.
Re: [classlib][modularity] Logging performance improvements (HARMONY-6362)
Posted by Tim Ellison <t....@gmail.com>.
On 27/Oct/2009 16:32, Jesse Wilson wrote:
> On Tue, Oct 27, 2009 at 7:20 AM, Tim Ellison <t....@gmail.com> wrote:
>
>> The improvement in performance is impressive. What is the contribution
>> made by the use of the CopyOnWriteArrayList?
>
> CopyOnWriteArrayList is ideal for situations when the following are all
> true:
> - reads are frequent
> - writes are rare
> - reads and writes happen concurrently
>
> This is exactly the scenario for logger's handlers. Each logged message
> results in a call to getHandlers() for the logger itself and for each of its
> parent loggers. The handlers list changes rarely: at startup only for
> typical apps. And it needs to be concurrent, because the logger API promises
> thread safety.
I didn't mean to ask "Why is a CopyOnWriteArrayList useful?", rather
what did it contribute to your logging improvements, e.g. correctness
that would require tons of duplicate coding otherwise, or performance
numbers, etc. (however, see below)
>> This would be our first dependency on util.concurrent from the logging
>> module, and if the benefit is small then I'd prefer to see reduced
>> coupling between the modules rather than a small improvement in
>> performance.
>
> I don't think that the increased coupling really hurts us. Even if a project
> adopts only our logging module, such a project will presumably also have its
> own java.util.concurrent module.
It's that assumption that I want to ensure we are always thinking about.
The ability to consume modules from harmony without having to deal with
inter-module spaghetti code is worth preserving.
>> What is the smallest useful set of modules that we should declare are
>> necessary for a runtime, so expect inter-module dependencies are ok; and
>> which ones are always going to be optional/left behind in smaller
>> runtimes? etc.
>>
>
> How do you feel about saying the core is "LUUUUUUUNI" ?
I'm prepared to accept that concurrent becomes part of the fundamental
core classes that are used to implement the remainder of the class
libraries. I'd be less happy about seeing lots of references to, say,
logging scattered throughout the other modules. We should remain modest
in our dependencies.
> - lang
> - util
> - util.concurrent.*
> - util.jar
> - util.regex
> - util.logging
> - util.prefs
> - util.zip
> - net
> - io
> This feels like a good fit to me, particularly since the RI exposes API
> interdependencies between these packages.
There's no accounting for bad design ;-}
Regards,
Tim
Re: [classlib][modularity] Logging performance improvements
(HARMONY-6362)
Posted by Jesse Wilson <je...@google.com>.
On Tue, Oct 27, 2009 at 7:20 AM, Tim Ellison <t....@gmail.com> wrote:
> The improvement in performance is impressive. What is the contribution
> made by the use of the CopyOnWriteArrayList?
CopyOnWriteArrayList is ideal for situations when the following are all
true:
- reads are frequent
- writes are rare
- reads and writes happen concurrently
This is exactly the scenario for logger's handlers. Each logged message
results in a call to getHandlers() for the logger itself and for each of its
parent loggers. The handlers list changes rarely: at startup only for
typical apps. And it needs to be concurrent, because the logger API promises
thread safety.
> This would be our first dependency on util.concurrent from the logging
> module, and if the benefit is small then I'd prefer to see reduced
> coupling between the modules rather than a small improvement in
> performance.
>
I don't think that the increased coupling really hurts us. Even if a project
adopts only our logging module, such a project will presumably also have its
own java.util.concurrent module.
What is the smallest useful set of modules that we should declare are
> necessary for a runtime, so expect inter-module dependencies are ok; and
> which ones are always going to be optional/left behind in smaller
> runtimes? etc.
>
How do you feel about saying the core is "LUUUUUUUNI" ?
- lang
- util
- util.concurrent.*
- util.jar
- util.regex
- util.logging
- util.prefs
- util.zip
- net
- io
This feels like a good fit to me, particularly since the RI exposes API
interdependencies between these packages.
Re: [classlib][modularity] Logging performance improvements
(HARMONY-6362)
Posted by Tony Wu <wu...@gmail.com>.
Good question. We may face more and more similar cases in future as
multi-core is the trend. an ideal solution is to detect if there is
concurrent module in classpath then we load it and use the improved
implementation otherwise we go back to the original.
On Tue, Oct 27, 2009 at 10:20 PM, Tim Ellison <t....@gmail.com> wrote:
> On 26/Oct/2009 20:33, Jesse Wilson (JIRA) wrote:
>> Logging performance improvements
>> --------------------------------
>>
>> Key: HARMONY-6362
>> URL: https://issues.apache.org/jira/browse/HARMONY-6362
>> Project: Harmony
>> Issue Type: Improvement
>> Components: Classlib
>> Environment: SVN Revision: 829932
>> Reporter: Jesse Wilson
>> Priority: Minor
>>
>>
>> The logging module performs more synchronization and object
>> allocation than necessary. In particular, getHandlers() always needs
>> synchronization and creates an array. By adopting util.concurrent
>> features (CopyOnWriteArrayList) and coding to the common case (when
>> there are zero handlers) I saw a 2.5x improvement in throughput for
>> calls to logger.log(Level, String). In a benchmark that measures calls
>> to "logger.info", call time improved from 385000ns to 154000ns per message.
>
> The improvement in performance is impressive. What is the contribution
> made by the use of the CopyOnWriteArrayList? I'll tell you why I am
> asking...
>
> This would be our first dependency on util.concurrent from the logging
> module, and if the benefit is small then I'd prefer to see reduced
> coupling between the modules rather than a small improvement in performance.
>
> Which is all part of the larger 'meta' question... What is the smallest
> set of modules that we see forming a useful runtime core? Today, people
> can reuse our logging implementation even if they don't have an
> implementation of concurrent. Should we relax that capability and
> assume that concurrent support is going to be universal?
>
> What is the smallest useful set of modules that we should declare are
> necessary for a runtime, so expect inter-module dependencies are ok; and
> which ones are always going to be optional/left behind in smaller
> runtimes? etc.
>
> Discuss :-)
> Tim
>
>
--
Tony Wu
China Software Development Lab, IBM
[classlib][modularity] Logging performance improvements (HARMONY-6362)
Posted by Tim Ellison <t....@gmail.com>.
On 26/Oct/2009 20:33, Jesse Wilson (JIRA) wrote:
> Logging performance improvements
> --------------------------------
>
> Key: HARMONY-6362
> URL: https://issues.apache.org/jira/browse/HARMONY-6362
> Project: Harmony
> Issue Type: Improvement
> Components: Classlib
> Environment: SVN Revision: 829932
> Reporter: Jesse Wilson
> Priority: Minor
>
>
> The logging module performs more synchronization and object
> allocation than necessary. In particular, getHandlers() always needs
> synchronization and creates an array. By adopting util.concurrent
> features (CopyOnWriteArrayList) and coding to the common case (when
> there are zero handlers) I saw a 2.5x improvement in throughput for
> calls to logger.log(Level, String). In a benchmark that measures calls
> to "logger.info", call time improved from 385000ns to 154000ns per message.
The improvement in performance is impressive. What is the contribution
made by the use of the CopyOnWriteArrayList? I'll tell you why I am
asking...
This would be our first dependency on util.concurrent from the logging
module, and if the benefit is small then I'd prefer to see reduced
coupling between the modules rather than a small improvement in performance.
Which is all part of the larger 'meta' question... What is the smallest
set of modules that we see forming a useful runtime core? Today, people
can reuse our logging implementation even if they don't have an
implementation of concurrent. Should we relax that capability and
assume that concurrent support is going to be universal?
What is the smallest useful set of modules that we should declare are
necessary for a runtime, so expect inter-module dependencies are ok; and
which ones are always going to be optional/left behind in smaller
runtimes? etc.
Discuss :-)
Tim
[jira] Closed: (HARMONY-6362) Logging performance improvements
Posted by "Jesse Wilson (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-6362?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jesse Wilson closed HARMONY-6362.
---------------------------------
Resolution: Fixed
Fixed with r833502.
> Logging performance improvements
> --------------------------------
>
> Key: HARMONY-6362
> URL: https://issues.apache.org/jira/browse/HARMONY-6362
> Project: Harmony
> Issue Type: Improvement
> Components: Classlib
> Environment: SVN Revision: 829932
> Reporter: Jesse Wilson
> Priority: Minor
> Original Estimate: 120h
> Remaining Estimate: 120h
>
> The logging module performs more synchronization and object allocation than necessary. In particular, getHandlers() always needs synchronization and creates an array. By adopting util.concurrent features (CopyOnWriteArrayList) and coding to the common case (when there are zero handlers) I saw a 2.5x improvement in throughput for calls to logger.log(Level, String). In a benchmark that measures calls to "logger.info", call time improved from 385000ns to 154000ns per message.
> Here's an overview of the patch:
> - Reduce the use of synchronization by adopting a CopyOnWriteArrayList for Handlers
> - Reduce object creation by reusing a 0-length handlers array in getHandlers()
> - Load handlers on logger creation rather than on receipt of the first message. This also fixes a behavioural inconsistency with the RI. I've got a test case that demonstrates this.
> - Cleanup synchronization by moving methods with "synchronized (LogManager.getManager())" blocks to the LogManager class
> I've got the patch ready to submit upstream. I'll submit it when my commit privileges are granted, unless anyone raises objections here.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HARMONY-6362) Logging performance improvements
Posted by "Tim Ellison (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-6362?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Tim Ellison updated HARMONY-6362:
---------------------------------
Fix Version/s: 5.0M12
> Logging performance improvements
> --------------------------------
>
> Key: HARMONY-6362
> URL: https://issues.apache.org/jira/browse/HARMONY-6362
> Project: Harmony
> Issue Type: Improvement
> Components: Classlib
> Environment: SVN Revision: 829932
> Reporter: Jesse Wilson
> Priority: Minor
> Fix For: 5.0M12
>
> Original Estimate: 120h
> Remaining Estimate: 120h
>
> The logging module performs more synchronization and object allocation than necessary. In particular, getHandlers() always needs synchronization and creates an array. By adopting util.concurrent features (CopyOnWriteArrayList) and coding to the common case (when there are zero handlers) I saw a 2.5x improvement in throughput for calls to logger.log(Level, String). In a benchmark that measures calls to "logger.info", call time improved from 385000ns to 154000ns per message.
> Here's an overview of the patch:
> - Reduce the use of synchronization by adopting a CopyOnWriteArrayList for Handlers
> - Reduce object creation by reusing a 0-length handlers array in getHandlers()
> - Load handlers on logger creation rather than on receipt of the first message. This also fixes a behavioural inconsistency with the RI. I've got a test case that demonstrates this.
> - Cleanup synchronization by moving methods with "synchronized (LogManager.getManager())" blocks to the LogManager class
> I've got the patch ready to submit upstream. I'll submit it when my commit privileges are granted, unless anyone raises objections here.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HARMONY-6362) Logging performance improvements
Posted by "Hudson (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-6362?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12774368#action_12774368 ]
Hudson commented on HARMONY-6362:
---------------------------------
Integrated in Harmony-1.5-head-linux-x86_64 #535 (See [http://hudson.zones.apache.org/hudson/job/Harmony-1.5-head-linux-x86_64/535/])
Fixing loggers to load handlers eagerly and track them with a CopyOnWriteArrayList. This improves performance and fixes a bug in our current implementation. A test for the bug is included; performance metrics are available in this JIRA:
https://issues.apache.org/jira/browse/
The change to CopyOnWriteArrayList also fixes some concurrency problems. Finally, I've addressed some minor readability issues. I intend to separate style from substance in forthcoming CLs...
> Logging performance improvements
> --------------------------------
>
> Key: HARMONY-6362
> URL: https://issues.apache.org/jira/browse/HARMONY-6362
> Project: Harmony
> Issue Type: Improvement
> Components: Classlib
> Environment: SVN Revision: 829932
> Reporter: Jesse Wilson
> Priority: Minor
> Original Estimate: 120h
> Remaining Estimate: 120h
>
> The logging module performs more synchronization and object allocation than necessary. In particular, getHandlers() always needs synchronization and creates an array. By adopting util.concurrent features (CopyOnWriteArrayList) and coding to the common case (when there are zero handlers) I saw a 2.5x improvement in throughput for calls to logger.log(Level, String). In a benchmark that measures calls to "logger.info", call time improved from 385000ns to 154000ns per message.
> Here's an overview of the patch:
> - Reduce the use of synchronization by adopting a CopyOnWriteArrayList for Handlers
> - Reduce object creation by reusing a 0-length handlers array in getHandlers()
> - Load handlers on logger creation rather than on receipt of the first message. This also fixes a behavioural inconsistency with the RI. I've got a test case that demonstrates this.
> - Cleanup synchronization by moving methods with "synchronized (LogManager.getManager())" blocks to the LogManager class
> I've got the patch ready to submit upstream. I'll submit it when my commit privileges are granted, unless anyone raises objections here.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.