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.

Sign up for GitHub

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

Jump to bottom

WindowsFileOperations: use JNI methods #2238

Closed
laszlocsomor opened this issue Dec 14, 2016 · 2 comments
Closed

WindowsFileOperations: use JNI methods #2238

laszlocsomor opened this issue Dec 14, 2016 · 2 comments
Assignees
@laszlocsomor
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) platform: windows type: feature request
Milestone
0.5

Comments

@laszlocsomor
Copy link
Contributor

According to my benchmark, JNI methods are faster than their Java native counterparts. I measured WindowsFileOperations.nativeIsJunction vs. the implementation in WindowsFileSystem.isJunction, and it's consistently faster.

(Measured everything twice to reveal any caching.)

  6000 distinct files (nonexistent, Java): 260 ms
  6000 distinct files (nonexistent, Java): 211 ms
  6000 distinct files (nonexistent, native): 88 ms
  6000 distinct files (nonexistent, native): 89 ms
  6000 distinct files (existent, Java): 426 ms
  6000 distinct files (existent, Java): 418 ms
  6000 distinct files (existent, native): 273 ms
  6000 distinct files (existent, native): 269 ms
  same file 6000 times (nonexistent, Java): 189 ms
  same file 6000 times (nonexistent, Java): 183 ms
  same file 6000 times (nonexistent, native): 64 ms
  same file 6000 times (nonexistent, native): 63 ms
  same file 6000 times (existent, Java): 241 ms
  same file 6000 times (existent, Java): 241 ms
  same file 6000 times (existent, native): 119 ms
  same file 6000 times (existent, native): 119 ms

Same is true for nativeGetLongPath (not yet committed), plus in that case the java native implementation is buggy.
@laszlocsomor laszlocsomor added P2 We'll consider working on this in future. (Assignee optional) type: feature request labels Dec 14, 2016
@laszlocsomor laszlocsomor self-assigned this Dec 14, 2016
@laszlocsomor
Copy link
Contributor Author

This must be especially true for WindowsFileSystem.createDirectoryJunction where we fork a new subprocess every time. I didn't measure the cost of this though.

bazel-io pushed a commit that referenced this issue Dec 20, 2016
@laszlocsomor @aehlig
WindowsFileSystem: use JNI impl. of isJunction
d40c78a 
Also build the JNI library while bootstraping.
This was once submitted in commit 4a249b6 but got
rolled back due to some test breakage that's long
since fixed. In this change I'm slightly modifying
the original code in compile.sh.

Using JNI methods however is necessary because we
can't implement WindowsFileOperations.GetLongPath
in native Java, and having that code is a
prerequisite for the fix of #2145

See also #2238

--
PiperOrigin-RevId: 142535019
MOS_MIGRATED_REVID=142535019
@laszlocsomor laszlocsomor added this to the 0.5 milestone Feb 14, 2017
bazel-io pushed a commit that referenced this issue Feb 15, 2017
@laszlocsomor @hermione521
Windows: use JNI CreateJunction in Java code
81f92fe 
Use the new CreateJunction in the Windows JNI code
every time we need to create junctions. This means
updating WindowsFileOperations and related tests.

Add test for WindowsFileSystem.createSymbolicLink.

See #2238

--
Change-Id: I5827e2e70e8e147f5f102fabf95fa9a148b3bcdc
Reviewed-on: https://cr.bazel.build/8896
PiperOrigin-RevId: 147598107
MOS_MIGRATED_REVID=147598107
@meteorcloudy meteorcloudy added P3 We're not considering working on this, but happy to review a PR. (No assignee) and removed P2 We'll consider working on this in future. (Assignee optional) labels Feb 28, 2017
@laszlocsomor
Copy link
Contributor Author

This has been fixed a long time ago.

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

@laszlocsomor laszlocsomor

Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) platform: windows type: feature request
Projects
None yet
Milestone
0.5
Development

No branches or pull requests
3 participants
@meteorcloudy @dslomov @laszlocsomor