harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Oliver Deakin <oliver.dea...@googlemail.com>
Subject Re: [jdktools][jdwp] Updated Java 6 JDWP contribution
Date Wed, 29 Apr 2009 14:54:46 GMT
Thanks Alexei - I was hoping to provide a more detailed description of 
the contribution but you have already done it for me :)

Alexei Fedotov wrote:
> Oliver,
> This is very interesting update. Nice job!
>
> One who looks into native.zip may suggest that the code is new, but in
> fact it looks like more an incremental change. I have noticed the
>   
Yes, this is based on the code that is currently in Harmony. I thought 
that providing the full set of files would probably be more readable 
than a diff, since there has been a fair amount of change across the 
whole code base.

> following and posting it here, so people knew what's happening:
> * 10 source files were added,
> * new Java 6 features were added as operations,
> * C++ exceptions were replaced with function return codes and
> function-like syntax,
> * 82 @author tags naming Aleksander Budniy, Anatoly Bondarenko,
> Viacheslav Rybalov, Anton Karnachuk, Pavel Vyssotski, Vitaly Provodin
> were removed (haven't we agreed keeping them as is? well, at least
> they can be found in this message)
>   

I couldn't remember if decided what to do with author tags. I went 
through tidying up the code a little for the contribution and removed 
the authors purely because I thought the code had had a large number of 
revisions since the originals were created, and that the @author tags 
were now no longer very useful or accurate. I can add them back in if 
anyone particularly minds them being removed.

> * usage of C strings were added,
> * logging macros were replaced from C++-like syntax to C syntax,
> * strange comments, e.g. /* Fix for 143846 - make sure we pass EBCDIC
> strings to zOS system functions */ appeared with no additional context
> on what is 143846
>   

This is a bug number - I thought I had replaced all of these with useful 
comments but obviously I've missed a couple ;)

> * the newly added code contains commented code, e.g.
> // printf("Looking for equivalent of java line %d in stratum %s\n",
> //        lineNumber, stratum);
>   or
> /*
>  ProcPtr jdwp::GetProcAddress(LoadedLibraryHandler libHandler, const
> char* procName)
> do we have a good reason to commit a commented code?
>   

Nope, I can trim these out when the contribution is committed.

> * there are several /* FIXME - Workaround for shutdown crashes */
> comments in the new code,
>   

Yes, there is a timing issue in the code which has been exposed by 
performance improvements. Very rarely during shutdown a crash can occur 
where monitors are freed whilst still being used by other threads. The 
agent should be waiting on those threads to complete before freeing 
monitors, but at the moment it does not for all threads. I believe this 
problem exists in the code that is currently in Harmony, but because of 
it's nature is not easily exposed. This is something to be fixed in the 
future but I'd like to do it in the open in Harmony, so for now I would 
commit with this workaround and log this as a JIRA for the future.

> * several doxygen comments start with an incorrect verb form (see
> http://java.sun.com/j2se/javadoc/writingdoccomments/#format for a
> correct one), the comments like /** A constructor. */ are not very
> helpful.
>
>   

Ok, Im not sure about this one as I don't know much about the format of 
doxygen comments. I have a feeling the unhelpful comments are probably 
automatically generated, and can be removed. These can (and should) be 
fixed up once the code is committed.

Regards,
Oliver

>
> On Wed, Apr 29, 2009 at 4:50 PM, Oliver Deakin
> <oliver.deakin@googlemail.com> wrote:
>   
>> Hi all,
>>
>> A little while ago, IBM created a mirror of the Java 6 jdktools branch so
>> that we could start bringing Harmony JDWP into the IBM Java releases. We
>> began working in that mirror because it was convenient for our internal
>> builds, with a plan to reflect all changes we made back into Harmony as we
>> went. Unfortunately, as deadlines drew in and releases came and went those
>> plans did not come into fruition - until now :)
>>
>> I'd like to announce the contribution of an enhanced Java 6 level JDWP agent
>> and socket transport layer. It has been attached to HARMONY-6187 [1] for
>> everyone to try out - please take a look. It has the following enhancements:
>> - A large number of bug fixes.
>> - Ported to a wide variety of platforms - Linux x86/x86_64, Linux PPC 32/64,
>> zLinux 31/64, Windows x86/x86_64, AIX PPC 32/64 and zOS 31/64.
>> - As part of the porting process, all non-portable C++ usage has been
>> removed and rewritten, particularly use of C++ standard libraries.
>> - JSR 45 support implemented. The new JDWP agent allows debugging of
>> non-Java stratum code running on the VM, for example JSPs.
>> - Significant performance improvements.
>>
>> All authors already have ACQs recorded. There may be some more work to be
>> done to get the new JDWP functioning 100% with Harmony, but I'd rather do
>> that work out in the open once it is committed. Please let me know if there
>> are any objections/comments to this contribution! I'll give it a few days
>> and then start a vote.
>>
>> Regards,
>> Oliver
>>
>> [1] https://issues.apache.org/jira/browse/HARMONY-6187
>>
>>
>> --
>> Oliver Deakin
>> Unless stated otherwise above:
>> IBM United Kingdom Limited - Registered in England and Wales with number
>> 741598. Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire
>> PO6 3AU
>>
>>
>>     
>
>
>
>   

-- 
Oliver Deakin
Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number 741598. 
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU


Mime
View raw message