Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Possible memory leakage in v1.0.0 (related with neureka.devices.CustomDeviceCleaner ) #26

Open
sunjianxy301 opened this issue Dec 3, 2024 · 13 comments
Assignees
Labels
bug Something isn't working

Comments

@sunjianxy301
Copy link

I have started using neureka 1.0.0 in my java project recently. Thank you very much for sharing this great project. I in desperate need of your help with solving a possible memory leakage. OutOfMemoryError will be raised as neureka functions calls grows. The following message from eclipse might help:

One instance of neureka.devices.CustomDeviceCleaner loaded by sun.misc.Launcher$AppClassLoader @ 0x3c0013e70 occupies 16,455,983,640 (99.20%) bytes. The memory is accumulated in one instance of java.lang.Object[], loaded by , which occupies 16,455,983,584 (99.20%) bytes.
Thread java.lang.Thread @ 0x430d0b2e0 Thread-10 has a local variable or reference to neureka.devices.CustomDeviceCleaner @ 0x3e03d0818 which is on the shortest path to java.lang.Object[6153400] @ 0x6e4ee2580. The thread java.lang.Thread @ 0x430d0b2e0 Thread-10 keeps local variables with total size 424 (0.00%) bytes.
Significant stack frames and local variables
neureka.devices.CustomDeviceCleaner.run()V (CustomDeviceCleaner.java:52)
neureka.devices.CustomDeviceCleaner @ 0x3e03d0818 retains 16,455,983,640 (99.20%) bytes
The stacktrace of this Thread is available. See stacktrace. See stacktrace with involved local variables.

My program was running on JDK 8 (Intel CPU, Windows 11 x64) .

@sunjianxy301
Copy link
Author

Maybe I should call MutableTensor.delete() for each Tensor. My problem should be solved.

@sunjianxy301
Copy link
Author

I've just found calling delete() on Tensor Object does not solve the problem. There are still a huge number of neureka.devices.ReferenceCounter, CustomDeviceCleaner$ReferenceWithCleaup object in my heap after i tried to call delete() on each Tensor used in the computing.

@sunjianxy301 sunjianxy301 reopened this Dec 4, 2024
@sunjianxy301
Copy link
Author

sunjianxy301 commented Dec 4, 2024

It seemed to me that the field 'list' of CustomDeviceCleaner has never been cleared and it could just keep growing.

final class CustomDeviceCleaner implements DeviceCleaner, Runnable
{
    private final ReferenceQueue<Object> _referenceQueue = new ReferenceQueue<>();
    private final long _timeout = 60 * 1000;
    private int _registered = 0;

    List<Object> list = new ArrayList<>();

    static class ReferenceWithCleanup<T> extends PhantomReference<T>
    {
        private final Runnable _action;

        ReferenceWithCleanup(T o, Runnable action, ReferenceQueue<T> queue) {
            super( o, queue );
            _action = action;
        }
        public void cleanup() {
            _action.run();
        }
    }

    @Override
    public void register(Object o, Runnable action) {
        synchronized ( _referenceQueue ) {
            list.add(new ReferenceWithCleanup<Object>(o, action, _referenceQueue));
            _registered++;
            if ( _registered == 1 ) new Thread( this::run ).start();
        }
    }
...
}

@Gleethos Gleethos self-assigned this Dec 9, 2024
Copy link

@Gleethos Gleethos added the bug Something isn't working label Dec 9, 2024
@Gleethos
Copy link
Owner

Gleethos commented Dec 9, 2024

Thank you so much for finding and reporting this issue!
I will push a fix soon.

@sunjianxy301
Copy link
Author

Great! Thank you for responding so quickly

@Gleethos
Copy link
Owner

I merged a hand full of general improvements from another branch which
already includes a rewrite of the device cleaner.
Based on my testing this should not produce any memory leaks.
A similar design is used in other projects as well. So it should be ok.

But can you verify this using the latest commit hash from main and verify in your code as well?
If you are using Gradle, you can check out the snapshot like this:

allprojects {
	repositories {
		//...
		maven { url 'https://jitpack.io' }
	}
}
// ...
dependencies {
	implementation 'com.github.Gleethos:neureka:241a79873c1c1da817a792282168af3914be9d6c' //main hash
}

I want to make sure that this is really fixed before I make a full release.
So it would be of great great help if you could approve the changes.

@sunjianxy301
Copy link
Author

sunjianxy301 commented Dec 13, 2024 via email

@sunjianxy301
Copy link
Author

Confirmed. No memory leakage. FYI, I have to use it like this:

    <dependency>
        <groupId>com.github.Gleethos</groupId>
        <artifactId>neureka</artifactId>
        <version>241a79873c1c1da817a792282168af3914be9d6c</version>
        <exclusions>
            <exclusion>
                <groupId>org.slf4j</groupId>
                <artifactId>slf4j-api</artifactId>
            </exclusion>
            <exclusion>
                <groupId>org.jetbrains.kotlin</groupId>
                <artifactId>kotlin-stdlib-jdk7</artifactId>
            </exclusion>
            <exclusion>
                <groupId>org.jetbrains.kotlin</groupId>
                <artifactId>kotlin-stdlib-jdk8</artifactId>
            </exclusion>
        </exclusions>
    </dependency>

I am using JAVA 8, I wonder if the coming release will not depend on kotlin stdl libs and a snapshot version slf4j-api. Anyway, thank you. Great project!

@Gleethos
Copy link
Owner

Gleethos commented Dec 14, 2024

Currently I have the following dependencies configured:

    implementation group: 'com.google.errorprone', name: 'error_prone_annotations', version: '2.27.0'
    implementation group: 'org.jspecify', name: 'jspecify', version: '1.0.0'
    implementation 'org.jocl:jocl:2.0.5'
    implementation group: 'org.slf4j', name: 'slf4j-api', version: '[1+, )'

This is different from before in that now the slf4j requirement is now more lenient,
and there are two small annotation libraries for static analysis...

Kotlin is only used in the test suite.

I hope that is compatible with your usage sites!

@Gleethos
Copy link
Owner

You have to exclude kotlin-stdlib for it to work?
That should not be necessary. This dependency should not be part of neureka.

@sunjianxy301
Copy link
Author

You have to exclude kotlin-stdlib for it to work? That should not be necessary. This dependency should not be part of neureka.

I saw kotlin-stdlib in the result of IntellijIdea's Analyze Dependencies function. It is not a big deal. I can remove it.

@Gleethos
Copy link
Owner

Gleethos commented Dec 16, 2024

I dispatched the release as version 1.0.1. Thank you again for the bug report and also
thank you for using Neureka, I hope it continues to provide value for you and
the projects you use it in.

I am using JAVA 8

Oh and just for your information, Neureka is also fully upwards compatible, so
if you are not using it in a legacy project stuck on Java 8, feel free
to use newer Java versions like 21.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

When branches are created from issues, their pull requests are automatically linked.

2 participants