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

[FR] readOpenMemory variant with pointer and size to work with mmap #2

Closed
FooIbar opened this issue Jun 23, 2024 · 16 comments
Closed
Assignees
Labels
enhancement New feature or request

Comments

@FooIbar
Copy link

FooIbar commented Jun 23, 2024

No description provided.

@zhanghai
Copy link
Owner

It seems to me you can implement it normally with the archive_read_open call - see also https://github.com/bramp/libarchive/blob/master/libarchive/archive_read_open_memory.c .

@zhanghai zhanghai self-assigned this Jun 23, 2024
@zhanghai zhanghai added the enhancement New feature or request label Jun 23, 2024
@FooIbar
Copy link
Author

FooIbar commented Jun 23, 2024

It seems to me you can implement it normally with the archive_read_open call - see also https://github.com/bramp/libarchive/blob/master/libarchive/archive_read_open_memory.c .

ReadCallback requires a ByteBuffer as well.

@Nullable SeekCallback<?> callback) throws ArchiveException;

BTW is the ? a typo?

@zhanghai
Copy link
Owner

ReadCallback requires a ByteBuffer as well.

What about MappedByteBuffer?

BTW is the ? a typo?

Ah yes, I think it's a typo.

@FooIbar
Copy link
Author

FooIbar commented Jun 23, 2024

ReadCallback requires a ByteBuffer as well.

What about MappedByteBuffer?

FileChannel.map has some annoying limitations, like, the size must not exceed Integer.MAX_VALUE (i.e. 2 GB).

zhanghai added a commit that referenced this issue Jun 24, 2024
@zhanghai
Copy link
Owner

zhanghai commented Jun 24, 2024

I can add an Archive.readOpenMemoryUnsafe() that takes a raw pointer and size. Out of curiosity, why do you need to mmap() the file instead of using regular IO (or pread())?

Meanwhile I tried but couldn't easily add an Archive.writeOpenMemoryUnsafe() because it's a bit awkward for how to return the actually used memory size in an async way without the ByteBuffer. So I hope you aren't doing mmap() for writes :P


Note to myself: https://bugs.openjdk.org/browse/JDK-6347833 - We could have used MemorySegment if and when our minimum SDK version supported Java FFM API (JEP 454).

@zhanghai
Copy link
Owner

Actually I think I can add the Archive.writeOpenMemoryUnsafe() because I can & should add a Java API for getting the used out-parameter any time.

@FooIbar
Copy link
Author

FooIbar commented Jun 24, 2024

Thanks!
I need to read multiple entries from the archive simultaneously.
pread() does work to some extent, but mmap() woud be better.

@zhanghai
Copy link
Owner

zhanghai commented Jun 24, 2024

I need to read multiple entries from the archive simultaneously.

Out of curiousity, how did you manage to do that? I thought libarchive isn't thread safe by itself:

The library is not thread aware, however. It does no locking or thread management of any kind. If you create a libarchive object and need to access it from multiple threads, you will need to provide your own locking.

https://github.com/libarchive/libarchive/blob/master/README.md

(Closing because the new APIs have been released in v1.1.0 - but please feel free to keep commenting.)

@FooIbar
Copy link
Author

FooIbar commented Jun 25, 2024

I read through the archive to get the entry list first, then use individual archive objects for requested entries.

@zhanghai
Copy link
Owner

zhanghai commented Jun 25, 2024

then use individual archive objects for requested entries.

  1. Does that mean you are creating multiple archive objects with archive_read_open_memory in order to read in parallel?
  2. How are you reusing the entries read earlier in order to read its data? I can imagine one can keep calling archive_read_next_header until the entry name matches with the desired entry, and do this in parallel. But maybe you know some better way to skip directly to the desired entry?

@FooIbar
Copy link
Author

FooIbar commented Jun 25, 2024

1) Yes 2) That's what I did

Here is the commit FooIbar/tachiyomi@35f5f0d

@zhanghai
Copy link
Owner

I see. I was looking at the other apps of yours which is using mmap() and madvise() with its own build of libarchive.

And curious, will you be upstreaming FooIbar/tachiyomi@35f5f0d to Mihon? (If that's the case I'll be happy that my code is part of Mihon as well - which I'm using right now.)

@FooIbar
Copy link
Author

FooIbar commented Jun 25, 2024

See mihonapp/mihon#949

@zhanghai
Copy link
Owner

Nice! Subscribed to that PR.

@AntsyLich
Copy link

If that's the case I'll be happy that my code is part of Mihon as well - which I'm using right now.

I use your file manager too 🙃

@zhanghai
Copy link
Owner

zhanghai commented Jun 25, 2024

I use your file manager too 🙃

Glad you like it 🤠

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants