Why is fork1() exported from hotspot?

[Volker Simonis]

The change for "6588413: Use -fvisibility=hidden for gcc compiles" had
a very long history and review thread (both open and private). The
performance tests done by Tom and me have absolutely no relationship
to the current question. They were about whether '-fvisibility=hidden'
could really improves the performance of the VM (as opposed to just
using map files for the control of exported symbols) and the answer
"yes". The VM performance indeed improved significantly on some
platforms, because '-fvisibility=hidden' allows the compile to
generate faster calls for local (i.e. not-exported) symbols. This
effect can not be achieved with map-files which are a link-time
feature.

Once we figured this out and convinced everybody that it is a good
thing to do, JNIEXPORT needed to be changed to contain
"__attribute__((visibility("default")))" otherwise it would not have
been possible to export symbols trough the map files because
'-fvisibility=hidden' is stronger (i.e. it overrides) the map file
settings (again, because the first one is a compile time while the
second one a link time feature). Finally, some of the functions which
needed to be exported (e.g. jio_printf) and hadn't been flagged as
JNIEXPORT, now had to be annotated with JNIEXPORT.

The changeset 6588413 did all this and it also did a little bit of
cleanup, like for example the removal of the old "green threads"
functions, which weren't needed any more. "fork1()" was one of these
functions and I'm pretty sure the intention was to remove it as well.
It actually was removed in one of the webrevs - see these comments on
the list (http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2011-January/001746.html):

=====

> Why did you remove fork1 from the mapfile?  other than that it looks good.

Because it was with the other old entry points and forgot to add it back
to the mapfile when I realized it had some mysterious numa use.

=====

Probably, the proper answer would have been "because it isn't needed
any more" but instead it unfortunately slipped back into the patch
together with the completely unrelated NUMA functions.

My advice - just delete it from both, the source and the map files.

And probably we should also remove numa_warn/numa_error if nobody
knows a really good reason to keep it. There's no point of saving all
this old crap in the source if there isn't anybody who knows what it
is good for.

Regards,
Volker


On Wed, Oct 21, 2015 at 5:32 PM, Daniel D. Daugherty
<daniel.daugherty at oracle.com> wrote:
> On 10/20/15, 1:14 AM, Thomas Stüfe wrote:
>>
>> Hi all,
>>
>> on Linux we define and export "fork1()" - a stub for fork() - and "fork1"
>> also appears in linker mapfiles for bsd and AIX. The latter, I am sure, is
>> just a copy-paste-effect.
>>
>> Why do we need to define and export fork1() for non-solaris platforms? We
>> only ever use it on Solaris.
>>
>> The comment in os_linux.cpp is not really enlightening:
>> "// Something to do with the numa-aware allocator needs these symbols"
>>
>> Does anyone know why this is needed?
>>
>> Regards, Thomas
>
>
> $ hg -R hotspot annot hotspot/src/os/linux/vm/os_linux.cpp | grep fork1
> 2072: extern "C" JNIEXPORT int fork1() { return fork(); }
>
> $ hg -R hotspot log -r 2072 hotspot/src/os/linux/vm/os_linux.cpp
> changeset:   2072:d70fe6ab4436
> parent:      2062:3582bf76420e
> user:        coleenp
> date:        Tue Feb 01 11:23:19 2011 -0500
> summary:     6588413: Use -fvisibility=hidden for gcc compiles
>
> Here's the review thread:
> http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2011-January/001726.html
>
>
> Here's the diff for just that file:
>
> $ hg -R hotspot diff -r 2062 -r 2072 hotspot/src/os/linux/vm/os_linux.cpp
> diff -r 3582bf76420e -r d70fe6ab4436 src/os/linux/vm/os_linux.cpp
> --- a/src/os/linux/vm/os_linux.cpp      Thu Jan 27 16:11:27 2011 -0800
> +++ b/src/os/linux/vm/os_linux.cpp      Tue Feb 01 11:23:19 2011 -0500
> @@ -2509,8 +2509,10 @@ char *os::scan_pages(char *start, char*
>    return end;
>  }
>
> -extern "C" void numa_warn(int number, char *where, ...) { }
> -extern "C" void numa_error(char *where) { }
> +// Something to do with the numa-aware allocator needs these symbols
> +extern "C" JNIEXPORT void numa_warn(int number, char *where, ...) { }
> +extern "C" JNIEXPORT void numa_error(char *where) { }
> +extern "C" JNIEXPORT int fork1() { return fork(); }
>
>
>  // If we are running with libnuma version > 2, then we should
> @@ -3483,7 +3485,7 @@ bool os::is_interrupted(Thread* thread,
>  // Note that the VM will print warnings if it detects conflicting signal
>  // handlers, unless invoked with the option "-XX:+AllowUserSignalHandlers".
>  //
> -extern "C" int
> +extern "C" JNIEXPORT int
>  JVM_handle_linux_signal(int signo, siginfo_t* siginfo,
>                          void* ucontext, int abort_if_unrecognized);
>
> @@ -4678,44 +4680,6 @@ void os::pause() {
>    }
>  }
>
> -extern "C" {
> -
> -/**
> - * NOTE: the following code is to keep the green threads code
> - * in the libjava.so happy. Once the green threads is removed,
> - * these code will no longer be needed.
> - */
> -int
> -jdk_waitpid(pid_t pid, int* status, int options) {
> -    return waitpid(pid, status, options);
> -}
> -
> -int
> -fork1() {
> -    return fork();
> -}
> -
> -int
> -jdk_sem_init(sem_t *sem, int pshared, unsigned int value) {
> -    return sem_init(sem, pshared, value);
> -}
> -
> -int
> -jdk_sem_post(sem_t *sem) {
> -    return sem_post(sem);
> -}
> -
> -int
> -jdk_sem_wait(sem_t *sem) {
> -    return sem_wait(sem);
> -}
> -
> -int
> -jdk_pthread_sigmask(int how , const sigset_t* newmask, sigset_t* oldmask) {
> -    return pthread_sigmask(how , newmask, oldmask);
> -}
> -
> -}
>
>  // Refer to the comments in os_solaris.cpp park-unpark.
>  //
>
>
> So fork1() used to be an "extern C" wrapper around fork() on Linux
> and now it is a JNIEXPORT'ed wrapper.
>
> I don't think the addition of this comment:
>
> +// Something to do with the numa-aware allocator needs these symbols
>
> was meant to apply to the new JNIEXPORT of fork1(). However, it
> seems very strange that fork1() moved from a section that was
> associated with "green threads" to a section associated with
> "numa-aware allocator"...
>
> Volker and Tom R are credited with doing some perf runs in the
> code review thread... Perhaps either of them can shed some light
> here?
>
> Dan