Review request 6981484: Update development launcher

[Coleen Phillimore]

Hi,

I think we should use the jdk7 version that you have in the windows 
build for all platforms. Many/most of the launcher jdk7 changes seem to 
be introducing the JLI layer and fixing some bugs. The platform 
dependent java_md.{ch} can be in os/<os>/launcher, and the java.c (which 
is the most code) in share/vm/tools/launcher - there isn't really risk 
to changing the solaris and linux versions since most people don't use 
them except for debugging. The only change that people would notice is 
if the libjvm.so is no longer statically linked in, because this is the 
primary benefit:

dbx gamma
stop in <some vm function> # no complaints from the debugger because 
libjvm.so is loaded already
run
<stop at break> # except this doesn't seem to work on linux now anyway

I can forsee using the launcher for development testing so having the 
latest jdk7 version makes a lot of sense, and having them consistent by 
platforms would be good going forward. I hope this isn't a ton of work 
to do though.

thanks,
Coleen

On 09/22/10 14:59, Staffan Larsen wrote:
> Thanks for the review, Coleen.
>
> Yes, the files are almost identical and we should really have just one set. There were two main reasons why I duplicated the code, and neither of them is very good:
>
> 1) The code in /os/(linux|solaris)/launcher/* is based on the launcher in 1.6.0-b28 (at least the comment in java.c says so). I couldn't easily find that source code to grab the windows files from, so I got them from some other build (6.0u22-b01). There was enough difference between these to make the platform independent and platform dependent files incompatible. 
> 2) I didn't want to make more changes than I had to, so I didn't want to touch the linux or solaris launchers.
>
> Perhaps there is even a third reason: There was already a precedent of having the linux and solaris code duplicated, so I just did the same thing for windows. Seems we would really benefit from a "posix" branch with common linux and solaris code in - in fact, the platform dependent parts of the launcher are the same for linux and solaris.
>
> In retrospect, none of these reasons are good enough, and we should use the same code as much as possible. Any thoughts on what version on the launcher I should use as baseline? The latest one from jdk7? A stable one from jdk6?
>
> Thanks,
> /Staffan
>
>
> On 22 sep 2010, at 16.20, Coleen Phillimore wrote:
>
>   
>> Hi Staffan,
>> Is this file src/os/windows/launcher/java.c
>> the same as ./os/linux/launcher/java.c and ./os/solaris/launcher/java.c
>> In fact, the sources in os/linux/launcher are almost exactly the same as os/solaris/launcher.
>> Can we have just one set?  Perhaps put it in directory src/share/tools/launcher and make the makefiles use these sources instead? Sorry to make more work but having the same 2000 lines of code somewhere else, guarantees one copy will always be wrong.
>>
>> Thanks,
>> Coleen
>>
>> On 09/22/10 08:38, Staffan Larsen wrote:
>>     
>>> Here we go again: http://cr.openjdk.java.net/~sla/6981484/webrev.03/
>>>
>>>
>>> Eventually, I'll get there...
>>>
>>> Thanks,
>>> /Staffan
>>>
>>> -----Original Message-----
>>> From: Christian Thalinger 
>>> Sent: den 22 september 2010 2:38
>>> To: Staffan Larsen
>>> Cc: 
>>> hotspot-dev at openjdk.java.net
>>>
>>> Subject: RE: Review request 6981484: Update development launcher
>>>
>>> On Wed, 2010-09-22 at 05:23 -0700, Staffan Larsen wrote:
>>>   
>>>
>>>       
>>>> Thanks Christian - I obviously have some things to learn as a new
>>>> committer - didn't think about the copyrights :-)
>>>>     
>>>>
>>>>         
>>> Sure.  I was going the same route :-)
>>>
>>>   
>>>
>>>       
>>>> http://cr.openjdk.java.net/~sla/6981484/webrev.02/
>>>>
>>>>
>>>> Now with updated copyrights.
>>>>     
>>>>
>>>>         
>>> As David already said.
>>>
>>> -- Christian
>>>
>>>   
>>>
>>>       
>
>   

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-dev/attachments/20100922/b1d344fc/attachment.html