Very poor performance of JNI AttachCurrentThread on Linux

[Dean Long]

This function should only get called in debug builds or for the initial 
thread, so I'm guessing
the app has a custom launcher because, if I'm not mistaken, the default 
launcher creates
a separate main thread to attach to the JVM while the initial thread waits.

But even so,  I don't think we need this function.  We already captured 
the bounds of the
initial thread in os::Linux::capture_initial_stack(), and the current 
range can be read using
getrlimit().

However if we do decide to keep get_stack_bounds(), I don't see why it 
can't use stdio like
find_vma().

dl

On 2/26/2013 9:58 AM, Andrew Haley wrote:
> get_stack_bounds() was rewritten because of a small memory leak.
> Instead of simply free()ing the memory to prevent the leak, it was
> rewritten to use a byte-by-byte loop around read() :
>
> http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2011-February/001864.html
>
> Unfortunately, the performance impact of this change is tragic.  As
> you can imagine, tens of thousands of system calls are made whenever
> get_stack_bounds() is called.
>
> Before rewrite: typically 100 microseconds
> After rewrite: typically 1500 microseconds
>
> It's impossible for me to tell from the discussion on the mailing list
> why such a change was made.  There is a rather elliptical comment
>
>> I'm strictly against reading /proc entry using stdio functions,
>> as (a) /proc file could be changed while we are reading it (b) it's not
>> fancy as we are buffering kmem.
> but this doesn't make any sense.  If the contents of "/proc/self/maps"
> really did change while it was being read, reading a byte at a time
> wouldn't help at all.  I don't know what the second sentence means.
>
> It would be possible to read "/proc/self/maps" in chunks and scan that,
> but my measurements show that it would not be significantly faster
> than the original version of get_stack_bounds().
>
> This severe regression is impacting a current large Java deployment.
> See https://bugzilla.redhat.com/show_bug.cgi?id=902004
>
> Is there any reason why I should not simply submit a webrev that reverts
> to the original code, with suitable use of free() ?  I have attached the
> code that I am testing.
>
> Andrew.
>
>
>
>
> static bool
> get_stack_bounds(uintptr_t *bottom, uintptr_t *top)
> {
>    FILE *f = fopen("/proc/self/maps", "r");
>    if (f == NULL)
>      return false;
>    char *str = NULL;
>    while (!feof(f)) {
>      size_t dummy;
>      ssize_t len = getline(&str, &dummy, f);
>      if (len == -1) {
>        free(str);
>        fclose(f);
>        return false;
>      }
>
>      if (len > 0 && str[len-1] == '\n') {
>        str[len-1] = 0;
>        len--;
>      }
>
>      static const char *stack_str = "[stack]";
>      if (len > (ssize_t)strlen(stack_str)
> 	&& (strcmp(str + len - strlen(stack_str), stack_str) == 0)) {
>        if (sscanf(str, "%" SCNxPTR "-%" SCNxPTR, bottom, top) == 2) {
>          uintptr_t sp = (uintptr_t)__builtin_frame_address(0);
>          if (sp >= *bottom && sp <= *top) {
>            free(str);
>            fclose(f);
>            return true;
>          }
>        }
>      }
>    }
>    free(str);
>    fclose(f);
>    return false;
> }
>
>