harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From enh <...@google.com>
Subject Re: [classlib][modularity] Logging performance improvements (HARMONY-6362)
Date Thu, 12 Nov 2009 22:32:19 GMT
On Thu, Nov 12, 2009 at 02:44, Tim Ellison <t.p.ellison@gmail.com> wrote:
> On 09/Nov/2009 22:31, Jesse Wilson wrote:
>> On Mon, Nov 9, 2009 at 4:14 AM, Tim Ellison <t.p.ellison@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/

Mime
View raw message