Using java.nio.file.Path in JavaIoFIlesystem.

[taras....@gmail.com]

JavaIoFilesystem is using java.io.File for most of the methods which has a disadvantage of requiring conversion to java.nio.file.Path in some methods but more importantly it makes JavaIoFilesystem jimfs, which relies on java.nio.file.Path, impossible to use. On a related note, I've noticed that some methods, like delete can be replaced with standard library functions like Files.delete and getDirectoryEntries can be implemented using Files.list. I'm guessing that it's because this filesystem implementation had to support older Java versions. Does anyone have any objections to migrate or at least add an additional getNioPath method in addition to getIoFile that can be used by methods that operate on java.nio.file.Path instead of java.io.File?

[Ulf Adams]

Hi Taras,

our vfs predates java.nio.file, which should explain some of the differences. However, we're reluctant to switch to java.nio.file wholesale - we're routinely handling hundreds of thousands of paths, and memory consumption and performance are critical. We're actually planning some rather invasive changes to the current vfs.Path and PathFragment classes to significantly reduce the overhead, which we likely wouldn't be able to make with java.nio.file.

Another issue is that we support extensive profiling for the vfs, so it's imperative that we don't use two different file system abstractions.

We've also talked about removing exists(), isReadable(), isWritable(), and friends entirely. Even within Bazel, these should rarely, if ever, be used. Almost all Bazel code should delegate to FileFunction, which is tracked in Skyframe, and not talk to the file system directly.

Also note that we have our own InMemoryFileSystem, which may not be as good as jimfs, but seems to work fine for us.

Cheers,

-- Ulf

On Mon, Oct 23, 2017 at 4:47 AM, <taras....@gmail.com> wrote:

JavaIoFilesystem is using java.io.File for most of the methods which has a disadvantage of requiring conversion to java.nio.file.Path in some methods but more importantly it makes JavaIoFilesystem jimfs, which relies on java.nio.file.Path, impossible to use. On a related note, I've noticed that some methods, like delete can be replaced with standard library functions like Files.delete and getDirectoryEntries can be implemented using Files.list. I'm guessing that it's because this filesystem implementation had to support older Java versions. Does anyone have any objections to migrate or at least add an additional getNioPath method in addition to getIoFile that can be used by methods that operate on java.nio.file.Path instead of java.io.File?

--
You received this message because you are subscribed to the Google Groups "bazel-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to bazel-dev+unsubscribe@googlegroups.com.
To post to this group, send email to baze...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/bazel-dev/bdafbb1d-d2ec-4f19-bb35-267487a2b5a6%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[taras....@gmail.com]

Hi Ulf,

Thank you for your response and historical context. I'm sorry if I didn't communicate my question well - I didn't mean to suggest replacing vfs.Path with java.nio.file.Path - I think you've done a great job building a powerful and expressive domain model and it's definitely the right thing to do. I only meant that when implementing JavaIoFilesystem methods you frequently use java.nio.file.Path, but instead of directly creating a java.nio.file.Path an extra allocation is used everywhere file.toPath() is used. In other words I'm only talking about implementation details - not changes to APIs. The other advantage I see is that if java.nio.file.Path was used instead of java.io.File, it would be possible to implement InMemoryFilesystem by just overriding a newly suggested getNioPath() method and jimfs would do the rest, which has an advantage of having to maintain one fewer FS implementations in Bazel :)

I also have my own selfish interest in having getNioPath method instead of getIoFile - in Buck we have to implement Bazel's FS interface for Skylark support and currently, since we rely on jimfs for in-memory FS, we cannot reuse JavaIoFilesystem. I understand that it's not a good reason for you to consider any changes, but hopefully the reasons above (fewer allocations and fewer FS implementations to maintain) make sense to you.

Thank you,

Taras

On Monday, October 23, 2017 at 12:04:51 AM UTC-7, Ulf Adams wrote:

Hi Taras,

our vfs predates java.nio.file, which should explain some of the differences. However, we're reluctant to switch to java.nio.file wholesale - we're routinely handling hundreds of thousands of paths, and memory consumption and performance are critical. We're actually planning some rather invasive changes to the current vfs.Path and PathFragment classes to significantly reduce the overhead, which we likely wouldn't be able to make with java.nio.file.

Another issue is that we support extensive profiling for the vfs, so it's imperative that we don't use two different file system abstractions.

We've also talked about removing exists(), isReadable(), isWritable(), and friends entirely. Even within Bazel, these should rarely, if ever, be used. Almost all Bazel code should delegate to FileFunction, which is tracked in Skyframe, and not talk to the file system directly.

Also note that we have our own InMemoryFileSystem, which may not be as good as jimfs, but seems to work fine for us.

Cheers,

-- Ulf

On Mon, Oct 23, 2017 at 4:47 AM, <taras....@gmail.com> wrote:

JavaIoFilesystem is using java.io.File for most of the methods which has a disadvantage of requiring conversion to java.nio.file.Path in some methods but more importantly it makes JavaIoFilesystem jimfs, which relies on java.nio.file.Path, impossible to use. On a related note, I've noticed that some methods, like delete can be replaced with standard library functions like Files.delete and getDirectoryEntries can be implemented using Files.list. I'm guessing that it's because this filesystem implementation had to support older Java versions. Does anyone have any objections to migrate or at least add an additional getNioPath method in addition to getIoFile that can be used by methods that operate on java.nio.file.Path instead of java.io.File?

--
You received this message because you are subscribed to the Google Groups "bazel-dev" group.

To unsubscribe from this group and stop receiving emails from it, send an email to bazel-dev+...@googlegroups.com.

[Philipp Wollermann]

Hi Taras,

thanks a lot for the suggestions!

I'm happy to have a look at this and will get back to you. I've filed a feature request issue on GitHub to track this: https://github.com/bazelbuild/bazel/issues/3952

Cheers,

Philipp

To view this discussion on the web visit https://groups.google.com/d/msgid/bazel-dev/01658495-0a45-48e7-a36e-db4349f97478%40googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

--

Google Germany GmbH
Erika-Mann-Straße 33
80636 München

Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg

[taras....@gmail.com]

Hi Philipp,

Thanks for filing a request issue! I'll try to send a small PR to introduce a `getNioPath`.