On Tue, Apr 15, 2008 at 11:45 PM, Alexey Varlamov < [EMAIL PROTECTED]> wrote:
> 2008/4/16, Nathan Beyer <[EMAIL > PROTECTED]<https://mail.google.com/mail?view=cm&tf=0&[EMAIL PROTECTED]> > >: > > I think we need more consensus on this. Is everyone in agreement about > > dumping log4cxx? Is this patch the appropriate replacement? > > Well, the suggestion has positive responses from me, Xiao-Feng and > Ilya Berezhniuk, and no specifc opposing arguments. Let's wait last > 24h, tomorrow I'll commit if no objections. Someone has to be the bad guy. I don't really care to honest, but I do know that big changes generally have big side-effects, so I think it's better to have more consensus and discussion up front. As the size and effect of the change increases, so should the discussion and number of people involved increase. -Nathan > > -- > Alexey > > > > > -Nathan > > > > On Tue, Apr 15, 2008 at 5:40 AM, Alexei Fedotov > > <[EMAIL PROTECTED]<https://mail.google.com/mail?view=cm&tf=0&[EMAIL > > PROTECTED]>> > wrote: > > > Nathan, > > > Could you please tell if you still have concerns about HARMONY-5692? > > > > > > Thanks, Alexei > > > > > > On Mon, Apr 14, 2008 at 8:26 AM, Alexei Fedotov > > > > > > > > > <[EMAIL PROTECTED]<https://mail.google.com/mail?view=cm&tf=0&[EMAIL > > > PROTECTED]>> > wrote: > > > > Hello Nathan, > > > > Thanks for your tough questions. > > > > 1. > > > > I'm sorry for using "red herring". I honestly believe that having > two > > > > charset coding libraries is overkill and a good reason to throw > one of > > > > them away. > > > > > > > > 2. > > > > Here is a JNI specification [1]. Please, check a table below > > > > JNI_CreateJavaVM description. It describes how VM logging is > > > > initialized. This was not implemented in DRLVM, and is implemented > in > > > > [2]. > > > > > > > > 3. > > > > A big chunk of removed functionality is logging into separate > files. > > > > To my perception, this is not a frequent use case. The original > reason > > > > why original DRLVM engineers attached log4cxx to my original C/C++ > > > > DRLVM logger was our attempt to achieve "Apache synergy". > > > > > > > > 4. > > > > You asked for an anecdotal proof of the fact why smaller > executables > > > > may run faster. I have just invented one: smaller executables > better > > > > fit into a processor cache, produce less cache misses and hence > > > > decrease a bus load. For a particular case this should be > measured, > > > > and I don't have solid data yet. > > > > > > > > 5. > > > > There is one more argument which actually forced me to start > thinking > > > > into this direction: my shmmap component (a hybrid of shm and > mmap) > > > > was originally developed as a part of aprutil (because it is based > on > > > > apr_shm and apr_mmap). We already have four porting libraries: > port, > > > > portlib, apr and aprutil in Harmony. Removing one which is not > tightly > > > > coupled with other parts make it easier for people like me to > decide > > > > where it is natural to place our hand-made components. > > > > > > > > More questions? Suggestions? > > > > > > > > [1] > http://java.sun.com/j2se/1.5.0/docs/guide/jni/spec/invocation.html > > > > [2] https://issues.apache.org/jira/browse/HARMONY-5692 > > > > > > > > > > > > > > > > > > > > On Mon, Apr 14, 2008 at 6:39 AM, Nathan Beyer <[EMAIL > > > PROTECTED]<https://mail.google.com/mail?view=cm&tf=0&[EMAIL PROTECTED]>> > wrote: > > > > > On Sun, Apr 13, 2008 at 4:17 AM, Alexei Fedotov > > > > > > > > > > <[EMAIL PROTECTED]<https://mail.google.com/mail?view=cm&tf=0&[EMAIL > > > PROTECTED]>> > wrote: > > > > > > > > > > > Hello Nathan, > > > > > > Thanks for questions. > > > > > > > > > > > > There are many other places where charset (en/de)coders are > used, but > > > > > > we use icu4c there. > > > > > > > > > > I know, but your original post made it sound like this was the > big > > > > > reason for accepting this patch, but charset codec usage was a > red > > > > > herring - I really don't like it when people use such tactics. > Focus > > > > > on the real issue, please. > > > > > > > > > > > > > > > > > > > > > HARMONY-5692 is a replacement of log4cxx with > > > > > > custom logger and removing dependent aprutil and apr-iconv > which are > > > > > > not used any longer after this replacement. > > > > > > * The replacement decreases > > > > > > > windows_x86_msvc_debug/deploy/jdk/jre/bin/default/harmonyvm.dll size > > > > > > from 4079616 to 2961408. This would probably improve a > startup time. > > > > > > > > > > Any proof, even anecdotal to this claim? > > > > > > > > > > > > > > > > * The custom logger requires less lines of code than a > log4cxx > > > > > > wrapper, hence, is easier to maintain. > > > > > > * By specification VM logging should be done using > provided > > > > > > vfprintf. C++ logger adds two layers of conversion. vfprintf > layer wan > > > > > > not implemented, and implementing it would increase a number > of > > > > > > wrappers for log4cxx. > > > > > > > > > > Can you point to this bit of the specification? I don't > understand the > > > > > last bit of what you're claiming. There's a wrapper that > doesn't > > > > > exist, but needs to? > > > > > > > > > > > > > > > > > > > > > > Please, let me know if these arguments are enough for > log4cxx being removed. > > > > > > > > > > No, they aren't for me. The original DRLVM engineers used > log4xx for a > > > > > reason, what functionality are we giving up? > > > > > > > > > > > > > > > > > > > > > > > > > > > Note, apr itself remains, and it is pretty good piece of > code to my > > > > > > perception. I do not intend to invent pools or OS call > wrappers. We > > > > > > throw away the staff guys from apr does not allow to put > into their > > > > > > main module. :-) > > > > > > Thanks. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Sun, Apr 13, 2008 at 12:30 AM, Nathan Beyer < > [EMAIL PROTECTED]<https://mail.google.com/mail?view=cm&tf=0&[EMAIL > PROTECTED]>> > wrote: > > > > > > > Huh? This post doesn't sound like anything in > HARMONY-5692. We need > > > > > > > clear somethings up first. > > > > > > > > > > > > > > 1. apr-iconv is in the build as a dependency of log4cxx > > > > > > > 2. there is no use of apr-iconv for charset > encoding/decoding > > > > > > > 3. HARMONY-5692, AFAIU is replacement of log4cxx with > custom logger > > > > > > > implementation to reduce the build time > > > > > > > > > > > > > > What's the value of replacing log4cxx with a custom > logger? Reducing > > > > > > > the build time isn't enough for me - there have to be > other ways to > > > > > > > accomplish that without sub-system replacement. > > > > > > > > > > > > > > -Nathan > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Fri, Apr 11, 2008 at 10:37 PM, Alexei Fedotov > > > > > > > <[EMAIL > > > PROTECTED]<https://mail.google.com/mail?view=cm&tf=0&[EMAIL PROTECTED]>> > wrote: > > > > > > > > Hello, > > > > > > > > > > > > > > > > One may notice that both icu and apr-iconv are > currently used to build > > > > > > > > Harmony HDK. Both libraries address charset encoding > and decoding. > > > > > > > > Should we keep both of them or try to use the only > one? > > > > > > > > > > > > > > > > HARMONY-5692 [1] contains one possible "compilable" > answer to this > > > > > > > > question and related reasoning. Alexey Varlamov and > Tim suggested > > > > > > > > discussing the matters on dev@ list, so I created this > thread on the > > > > > > > > subject. You are welcome to share your opinion about > the libraries > > > > > > > > Harmony depends on, the patch from [2], my arguments > defending the > > > > > > > > patch, and further directions here. > > > > > > > > > > > > > > > > -- > > > > > > > > With best regards, > > > > > > > > Alexei > > > > > > > > > > > > > > > > [1] https://issues.apache.org/jira/browse/HARMONY-5692 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > With best regards, > > > > > > Alexei > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > With best regards, > > > > Alexei > > > > > > > > > > > > > > > > -- > > > With best regards, > > > Alexei > > > > > >
