JDK-8177969 : Faster FilePermission::implies by avoiding the use of Path::relativize
  • Type: Bug
  • Component: core-libs
  • Sub-Component: java.io
  • Affected Version: 9
  • Priority: P2
  • Status: Closed
  • Resolution: Fixed
  • Submitted: 2017-04-03
  • Updated: 2017-05-12
  • Resolved: 2017-04-11
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 10 JDK 9
10Fixed 9 b166Fixed
Related Reports
Relates :  
JDK-8164705 - Remove pathname canonicalization from FilePermission
Description
The new FilePermission::implies implementation introduced in JDK-8164705 uses Path::relativize to find out if a path is inside another one. Path::relativize is quite complicated and since we don't need the actual result (we only need to see if the result is zero-or-more ".."), we can avoid using this method and write our own check.
Comments
Final benchmark result: Label Benchmark Mode Cnt Score Error Units ------- --------------- ----- --- ---------- -------- ----- Unfixed checkReadNew thrpt 15 178343.748 �� 3457.535 ops/s Fixed00 checkReadNew thrpt 15 415512.031 �� 8135.591 ops/s Fixed01 checkReadNew thrpt 15 444520.258 �� 6866.461 ops/s JDK9 checkReadOld200 thrpt 15 369654.145 �� 9639.859 ops/s JDK9 checkReadOld201 thrpt 15 2034.845 �� 72.707 ops/s JDK8 checkReadOld200 thrpt 15 425876.205 �� 8042.674 ops/s JDK8 checkReadOld201 thrpt 15 2090.943 �� 180.692 ops/s The labels: - Unfixed: jdk9+161 unchanged - Fixed00: jdk9+161 with patches from http://cr.openjdk.java.net/~weijun/8177969/webrev.00/ - Fixed01: jdk9+161 with final patch - JDK9: jdk9+161 with -Djdk.io.permissionsUseCanonicalPath=true - JDK8: jdk8u131 b11 Old200 loops through 200 different paths so the canonicalized path cache holds all of them, and Old201 loops through 201 different paths so every path is new to the cache. The result is from a linux-x64 machine. The benchmark code is attached. It is compiled with a local JMH 1.19-SNAPSHOT built from http://hg.openjdk.java.net/code-tools/jmh/rev/36a2ee9a075e.
11-04-2017
This is a performance regression in 9. Sean and Roger have agreed to do a thorough review. So I think this is okay to fix in 9.
06-04-2017
Benchmark Mode Cnt Score Error Units MyBenchmark.checkReadNewUnfixed thrpt 5 160280.127 �� 5241.103 ops/s MyBenchmark.checkReadNewFixed thrpt 5 453278.086 �� 431342.923 ops/s MyBenchmark.checkReadOld200 thrpt 5 330838.183 �� 22281.658 ops/s MyBenchmark.checkReadOld201 thrpt 5 2005.148 �� 285.398 ops/s New means jdk9 behavior (-Djdk.io.permissionsUseCanonicalPath=false), and old means jdk 8 (=true). Old200 loops through 200 different paths so the canonicalized path cache holds all of them, and Old201 loops through 201 different paths so every path is new to the cache. Test runs with jdk9+161, NewFixed with --patch-module java.base=/path/to/classes/holding/updated/FilePermission.
06-04-2017
"Performance or functional regressions that only affect a limited part of the product or use cases" can be treated P2. In the case I am investigating, the time spent on the test is 78s (jdk8 behavior), 170s (jdk9 behavior), and 75s (jdk9 fixed).
05-04-2017
Max - are you sure this is a P2 and critical for JDK 9?
04-04-2017
Why it's important to fix this bug: The new check is much faster than Path::relativize. We have observed a 100% performance gain in a real world product. Explain the nature of the fix: The new check can be viewed as a simplified implementation of Path::relativize that only does what FilePermission::implies needs. Risk: Low. This is a refactoring of an existing method. No API change. The new method is quite clean. Test coverage: Existing tests Reviewers: TBD. Webrev: http://cr.openjdk.java.net/~weijun/8177969/webrev.00/
03-04-2017