This repository has been archived by the owner. It is now read-only.

#571 NativeImageLoader buffer reuse #573

Merged
merged 4 commits into from May 8, 2018

Conversation

Projects
None yet
3 participants
@AlexDBlack
Copy link
Member

AlexDBlack commented Apr 30, 2018

WIP DO NOT MERGE
Fixes: #571

All tests pass, good to go except for loading issue noted below (which should be resolved first).

@saudet without adding an Nd4j.create(1) at the top of the test (and 1 or 2 others now) I'm getting the following (when the tests are run in isolation):

java.lang.RuntimeException: No native JavaCPP library in memory. (Has Loader.load() been called?)
	at org.bytedeco.javacpp.BytePointer.<init>(BytePointer.java:103)
	at org.datavec.image.loader.NativeImageLoader.asMatrix(NativeImageLoader.java:211)
	at org.datavec.image.loader.NativeImageLoader.asMatrix(NativeImageLoader.java:202)
	at org.datavec.image.loader.TestNativeImageLoader.testBufferRealloc(TestNativeImageLoader.java:330)
...
Caused by: java.lang.UnsatisfiedLinkError: org.bytedeco.javacpp.BytePointer.allocateArray(J)V
	at org.bytedeco.javacpp.BytePointer.allocateArray(Native Method)
	at org.bytedeco.javacpp.BytePointer.<init>(BytePointer.java:98)
	... 25 more

What's the best workaround/solution here?

@AlexDBlack AlexDBlack requested a review from saudet Apr 30, 2018

int length = pair.getSecond();
BytePointer bp = new BytePointer(length);
bp.put(bytes, 0, length);
Mat mat = new Mat(bp, false);

This comment has been minimized.

@saudet

saudet Apr 30, 2018

Member

We don't need to create a BytePointer explicitly here, just call new Mat(bytes) like before and it should work.

BTW, creating a new BytePointer like that on each call is also bound to create a lot of garbage. You're not seeing that?

This comment has been minimized.

@AlexDBlack

AlexDBlack Apr 30, 2018

Member

I don't see how it can work like that new Mat(bytes) doesn't know that the buffer isn't entirely used.

At this point I'm just removing the byte[] garbage - and not so much the total number of objects... the creation of the BytePointer like that was taken from what happens internally in new Mat(bytes) anyway - there is no new Mat(bytes, int length) constructor or anything.

I'm not sure how we could re-use the BytePointer (without creating even more objects) when the length will change in most cases?

This comment has been minimized.

@saudet

saudet May 1, 2018

Member

Let me take a stab at it

@AlexDBlack

This comment has been minimized.

Copy link
Member

AlexDBlack commented May 8, 2018

@saudet Any progress here? Or should we merge this as is (to reduce the byte[] memory use) and look at the pointer/offheap component separately?

@saudet

This comment has been minimized.

Copy link
Member

saudet commented May 8, 2018

Ah yes let me do this now actually.

saudet added some commits May 8, 2018

@saudet

saudet approved these changes May 8, 2018

@agibsonccc agibsonccc merged commit 0472c38 into master May 8, 2018

1 check passed

continuous-integration/jenkins/pr-merge This commit looks good
Details

@agibsonccc agibsonccc deleted the ab_571_imgrr_buffer branch May 8, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.