7003707 - Re: 6348631 - request for review (updated)

[Ivan Krylov]

David,
I propose that you take edit 7003707 by either closing it or edit it per Coleen's suggestion to something like
Synopsis: review inlined methods and move where possible to corresponding cpp files
where word "possible" means no performance or build time costs.
It that reasonable?
Ivan

On 04.12.2010 9:37, David Holmes wrote:
> I think this is all a non-issue and the RFE is unnecessary. Here are the files Paul mentioned and I don't see anything that needs "fixing".
>
> hypert /mirrors/ws-mirrors/hsdev/hotspot/src > grepsrc.sh \#include | grep .hpp: | grep -v shark | grep \< | grep -v zero
> ./os/linux/vm/hpi_linux.hpp:#include <unistd.h>
> ./os/linux/vm/hpi_linux.hpp:#include <sys/socket.h>
> ./os/linux/vm/hpi_linux.hpp:#include <sys/poll.h>
> ./os/linux/vm/hpi_linux.hpp:#include <sys/ioctl.h>
> ./os/linux/vm/hpi_linux.hpp:#include <netdb.h>
> ./os/solaris/vm/hpi_solaris.hpp:#include <sys/socket.h>
> ./os/solaris/vm/hpi_solaris.hpp:#include <sys/poll.h>
> ./os/solaris/vm/hpi_solaris.hpp:#include <sys/filio.h>
> ./os/solaris/vm/hpi_solaris.hpp:#include <unistd.h>
> ./os/solaris/vm/hpi_solaris.hpp:#include <netdb.h>
> ./os/solaris/vm/hpi_solaris.hpp:#include <setjmp.h>
> ./os_cpu/linux_arm/vm/bytes_linux_arm.inline.hpp:#include <byteswap.h>
> ./os_cpu/linux_x86/vm/bytes_linux_x86.inline.hpp:#include <byteswap.h>
> ./share/vm/adlc/adlc.hpp:#include <iostream>
> ./share/vm/adlc/adlc.hpp:#include <sys/types.h>
> ./share/vm/adlc/adlc.hpp:  #include <inttypes.h>
> ./share/vm/adlc/filebuff.hpp:#include <iostream>
> ./share/vm/classfile/classLoader.hpp:#include <sys/stat.h>
> ./share/vm/gc_implementation/g1/ptrQueue.hpp:#include <new>
> ./share/vm/libadt/port.hpp:#include <stddef.h>
> ./share/vm/libadt/port.hpp:#include <stdlib.h>
> ./share/vm/libadt/port.hpp:#include <string.h>
> ./share/vm/libadt/port.hpp:#include <mem.h>
> ./share/vm/libadt/port.hpp:#include <string.h>
> ./share/vm/libadt/port.hpp:#include <strings.h>
> ./share/vm/libadt/port.hpp:#include <stdlib.h>
> ./share/vm/libadt/port.hpp:#include <stdlib.h>
> ./share/vm/libadt/port.hpp:#include <memory.h>
> ./share/vm/libadt/port.hpp:#include <stdlib.h>
> ./share/vm/libadt/port.hpp:#include <memory.h>
> ./share/vm/libadt/port.hpp:#include <stdlib.h>
> ./share/vm/libadt/port.hpp:#include <stddef.h>
> ./share/vm/libadt/port.hpp:#include <stdlib.h>
> ./share/vm/libadt/port.hpp:#include <string.h>
> ./share/vm/libadt/port.hpp:#include <stddef.h>
> ./share/vm/libadt/port.hpp:#include <stdlib.h>
> ./share/vm/libadt/port.hpp:#include <string.h>
> ./share/vm/libadt/port.hpp:#include <unistd.h>
> ./share/vm/libadt/port.hpp:// #include <bstring.h>
> ./share/vm/oops/typeArrayOop.hpp:#include <limits.h>
> ./share/vm/utilities/debug.hpp:#include <stdarg.h>
> ./share/vm/utilities/dtrace.hpp:#include <sys/sdt.h>
> ./share/vm/utilities/globalDefinitions_gcc.hpp:#include <ctype.h>
> ./share/vm/utilities/globalDefinitions_gcc.hpp:#include <string.h>
> ./share/vm/utilities/globalDefinitions_gcc.hpp:#include <stdarg.h>
> ./share/vm/utilities/globalDefinitions_gcc.hpp:#include <stddef.h>
> ./share/vm/utilities/globalDefinitions_gcc.hpp:#include <stdio.h>
> ./share/vm/utilities/globalDefinitions_gcc.hpp:#include <stdlib.h>
> ./share/vm/utilities/globalDefinitions_gcc.hpp:#include <wchar.h>
> ./share/vm/utilities/globalDefinitions_gcc.hpp:#include <ieeefp.h>
> ./share/vm/utilities/globalDefinitions_gcc.hpp:#include <math.h>
> ./share/vm/utilities/globalDefinitions_gcc.hpp:#include <time.h>
> ./share/vm/utilities/globalDefinitions_gcc.hpp:#include <fcntl.h>
> ./share/vm/utilities/globalDefinitions_gcc.hpp:#include <dlfcn.h>
> ./share/vm/utilities/globalDefinitions_gcc.hpp:#include <pthread.h>
> ./share/vm/utilities/globalDefinitions_gcc.hpp:#include <thread.h>
> ./share/vm/utilities/globalDefinitions_gcc.hpp:#include <limits.h>
> ./share/vm/utilities/globalDefinitions_gcc.hpp:#include <errno.h>
> ./share/vm/utilities/globalDefinitions_gcc.hpp:#include <sys/trap.h>
> ./share/vm/utilities/globalDefinitions_gcc.hpp:#include <sys/regset.h>
> ./share/vm/utilities/globalDefinitions_gcc.hpp:#include <sys/procset.h>
> ./share/vm/utilities/globalDefinitions_gcc.hpp:#include <ucontext.h>
> ./share/vm/utilities/globalDefinitions_gcc.hpp:#include <setjmp.h>
> ./share/vm/utilities/globalDefinitions_gcc.hpp:#include <inttypes.h>
> ./share/vm/utilities/globalDefinitions_gcc.hpp:#include <signal.h>
> ./share/vm/utilities/globalDefinitions_gcc.hpp:#include <ucontext.h>
> ./share/vm/utilities/globalDefinitions_gcc.hpp:#include <sys/time.h>
> ./share/vm/utilities/growableArray.hpp:#include <new>
>
> Let's move on.
>
> Cheers,
> David
>
> Coleen Phillimore said the following on 12/03/10 22:42:
>>
>>>> I agree. This sounds like the right approach, and one that is consistent
>>>> with past practice. This would also be exactly what the
>>>> make-deps removal accomplished for the existing .inline.hpp dependencies as
>>>> well.
>>>
>>> So just to be clear here, Volker and Ramki agree with me that the system includes should remain in the header files that need them - right? 
>>> Whereas Ivan and Coleen want to see them moved into the cpp files.
>>
>> No, I agree with that too.  I have no argument against the .hpp  or .inline.hpp  files containing the include directives that it needs.  In this 
>> hpi case, we were inlining functions that needed system includes.  My request was that the functions not be inlined so that the system includes can 
>> be included in the .cpp file instead, to minimize exposure of the entire source base to these system includes.   It was just to keep the gnarly 
>> implementation details as local as possible.
>>
>> thanks,
>> Coleen
>>>
>>> David
>>>
>>