Merging BSDPort into HotSpot mainline
Tom Rodriguez
tom.rodriguez at oracle.com
Tue Aug 2 08:47:39 PDT 2011
On Jul 28, 2011, at 10:17 AM, roger hoover wrote:
> As the first of many steps to integrate the Mac OS X Port into mainline jdk8, we need to merge the BSDPort code (on which the Mac OS X Port is based) into the jdk8 mainline. This message is intended to begin the discussion of that merge for HotSpot.
>
> To make the process manageable, we've looked through the diff and have attempted to break up the changes into some smaller pieces for better discussion and analysis. In particular, since many of the changes are modifications upon the linux port, we've included diffs between linux and bsd for those files as a way to understand the otherwise huge additions of new stuff.
I took the liberty of splitting your patch into 3 parts. The first is the #ifdef TARGET_ changes which are benign, the second is the rest of the changes to shared files and the third is all the new files. Webrevs for 1 and 2 are at http://cr.openjdk.java.net/~never/bsd_headers and http://cr.openjdk.java.net/~never/bsd_other respectively. I haven't generated a webrev for the 3rd part since it requires some trickery to produce it.
The header changes look fine. Here are the issues I see in the shared changes.
The agent and makefile changes look fine to me.
What are the UseMembar changes about? They are fine, I'm curious why they are needed. I believe !UseMembar is more efficient.
src/cpu/x86/vm/c1_LIRAssembler_x86.cpp:
src/cpu/x86/vm/interp_masm_x86_32.cpp:
Why is the change of the cast needed? That idiom is used in many other places that haven't been modified. It's an odd little idiom that could be eliminated by fixing a limitation in the MacroAssembler where it emits larger assembly than it needs to. Mainly I'm just unclear why it was changed in some places and not others.
src/share/vm/interpreter/bytecodeTracer.cpp:
Should this just be converted to INTPTR_FORMAT and intptr_t?
src/share/vm/runtime/os.cpp:
This is an odd little hack. Why is this in the JVM?
globalDefinitions.hpp:
INTPTR_FORMAT has been changed to a decimal format when it's supposed to be hexadecimal. I think it should be PRIxPTR instead.
These changes might cause compilation problems on earlier releases since for instance Solaris 8 doesn't have PRI*PTR. We might want to add something like:
#ifndef PRIxPTR
#ifdef _LP64
#define PRIxPTR PRIx64
#else
#define PRIxPTR PRIx32
#endif
I see a full copy of elf.h in there as well which I doubt we can accept. It seems to be apple specfic but doesn't mac use MachO?
test/Makefile:
What's the point of the JTREG_TESTDIRS change?
>
> I've created wiki pages off the BSDPort page that contains these diffs. Please add your comments and analysis to these pages so that It becomes a record of issues to be addressed in this merge. The root sub-page is:
> http://wikis.sun.com/display/OpenJDK/BSDPort%2C+Description+of+jdk7+changes
>
> We're going to need someone on Oracle's hotspot team to help with this. What is the next step? Do I file a bug or change request?
I don't know whether the runtime team will want to pursue some refactoring of the bsd changes so there isn't as much duplication or whether it will be ok to merge it as is and deal with refactoring later. That might affect how we wan to handle this.
tom
>
> roger
>
More information about the hotspot-dev
mailing list