Guys,
unfortunately the suggested patch does not work on Windows.
Here is the memory check code
=== cut ===
#define MEMORYLOADTHRESHOLD 95 // 95%
#define MEMORYFREETHRESHOLD 0x4000000 //64M
hysysinfo_is_system_physical_memory_low (struct HyPortLibrary
*portLibrary, UDATA load_threshold, UDATA free_threshold)
{
MEMORYSTATUSEX stat;
stat.dwLength = sizeof (stat);
if(GlobalMemoryStatusEx(&stat))
{
if(stat.dwMemoryLoad >= load_threshold || stat.ullAvailPhys <=
load_threshold){
return TRUE;
}
=== cut ===
GlobalMemoryStatusEx reports global memory status but not the status
for the current process. So we are waiting while full system memory
load will be higher then 95% or available physical memory will be less
the 95 bytes. I guess that this is usual situation on 512M or even 1G
system. But this is not true for my 4G server :) So this fuction
always return false for me. And OOM happens after 100 iterations while
running the reproducer.
I undestand that free_threshold and load_threshold parameters are
mixed up here. But I believe that putting them in place will not help.
So the issue needs additional investigation.
SY, Alexey
2007/5/23, Mikhail Markov <[EMAIL PROTECTED]>:
+1 for this approach.
As for removing OSResourcesMonitor - not agree: IMO we should have this
facility for another code not using OSMemory.malloc() method which wants to
check the conditions for calling GC.
Thanks,
Mikhail
On 5/23/07, Alexey Petrenko <[EMAIL PROTECTED]> wrote:
>
> I suggest to call nativeMalloc anyway. nativeMalloc will check the
> native heap and return null or something if gc is needed whithout real
> allocation.
> Java code will check the return value and call gc and nativeMalloc if
> needed.
>
> So the code will look like the following
>
> === cut ===
> public long malloc(long length) throws OutOfMemoryError {
> int res = mallocNative(length, true);
> if (res == 0) {
> System.gc();
> res = mallocNative(length, false);
> }
> return res;
> }
> === cut ===
>
> === cut ===
> JNIEXPORT jlong JNICALL
> Java_org_apache_harmony_luni_platform_OSMemory_mallocNative
> (JNIEnv * env, jobject thiz, jlong size, jboolean checkHeap)
> {
> PORT_ACCESS_FROM_ENV (env);
>
> if (checkHeap) {
> if (hysysinfo_is_system_physical_memory_low_default()) {
> return 0;
> }
> }
>
> void *address = hymem_allocate_memory ((UDATA) size);
>
> if (address == NULL)
> {
> throwNewOutOfMemoryError(env, "Insufficient memory available.");
> }
>
> return (jlong) ((IDATA) address);
> }
> === cut ===
>
> I think that this approach is better because:
> 1. In the good case we got up to 2 times less number of native calls.
> In the worst case we got the same number of native calls.
> 2. OSResourcesMonitor class becomes unneeded.
>
> SY, Alexey
>
>
> 2007/5/23, Mikhail Markov <[EMAIL PROTECTED]>:
> > If the method returns negative flag (should run GC) then after GC
> invocation
> > the malloc should be allocated, so the native method could 1) call GC
> from
> > native code (come back to java) and then alloc memory or 2) (this way is
> > implemented by the patch) return to java and java-code will call malloc
> > after GC.
> > Or you suggest to malloc before calling GC in any case?
> >
> > Thanks,
> > Mikhail
> >
> >
> > On 5/23/07, Alexey Petrenko <[EMAIL PROTECTED]> wrote:
> > >
> > > 2007/5/23, Alexei Zakharov <[EMAIL PROTECTED]>:
> > > > As far as I know DRLVM does not run on kernels older than 2.6 - see
> > > > old "building on Debian" thread for details [1]. So it looks like in
> > > > order to enable Harmony on pre2.6 (and on pre2.3) kernels we will
> need
> > > > to fix our code in several places anyway. And the
> > > > hysysinfo_is_system_physical_memory_low() function is not the most
> > > > complicated one.
> > > >
> > > > This way, IMHO current patches for HARMONY-3148 look quite good. And
> > > > since the issue itself is rather critical IMO it is not a bad idea
> to
> > > > finally have it fixed and commit patches in their current state. If
> > > > later we decide to use other strategy for native memory deallocation
> > > > then we can always create another JIRA & patch and fix this.
> > > >
> > > > BTW, Tim, if you don't have enough time for this (or do not
> interested
> > > > anymore) I could take care of it.
> > > I've took it already... :)
> > >
> > > I had a quick look into the patch and got one question...
> > > The patch creates a new class OSResourcesMonitor with the
> > > isSystemPhysicalMemoryLowDefault native method.
> > > OSMemory.malloc calls this method in the very begining and then call
> > > another native method to allocate memory.
> > >
> > > Why do we need two native calls here if we can check the native heap
> > > size in the native method which allocates it? This method can return a
> > > flag to run GC...
> > >
> > > Did I miss something?
> > >
> > > SY, Alexey
> > >
> >
>