Uri.collapseDotSegments fails on some relative paths #2507

[Yash0215]

Uri.collapseDotSegments fails on some relative paths #2507 Description: the conclusion is that the 'collapseDotSegments' doesnt work as described in 'http://tools.ietf.org/html/rfc3986#section-5.2.4' when relative path doent start with a '/'. the easy solution is to check whether the given Uri starts with a '/' or not. and call 'process' accordingly.

[lightbend-cla-validator]

Hi @Yash0215,

Thank you for your contribution! We really value the time you've taken to put this together.

Before we proceed with reviewing this pull request, please sign the Lightbend Contributors License Agreement:

https://www.lightbend.com/contribute/cla

[akka-ci]

Thank you for your pull request! After a quick sanity check one of the team will reply with 'OK TO TEST' to kick off our automated validation on Jenkins. This compiles the project, runs the tests, and checks for things like binary compatibility and source code formatting. When two team members have also manually reviewed and (perhaps after asking for some amendments) accepted your contribution, it should be good to be merged.

For more details about our contributing process, check out CONTRIBUTING.md - and feel free to ask!

[akka-ci]

Test FAILed.

!!! Couldn't read commit file !!!

[akka-ci]

Test PASSed.

[jrudolph]

Thanks, @Yash0215. This is looking good. I wonder where those URIs would turn up though, as file URIs in particular don't support relative paths according to https://tools.ietf.org/html/rfc8089.

That said, http://tools.ietf.org/html/rfc3986#section-5.2.4 which you refer to above, has an explicit example using a relative path, so it would probably make sense to support it correctly.

[Yash0215]

such types of URI raises "IllegalUriException" when host is present but are allowed when authority is empty for some reason. so I thought it is better to support it correctly instead of producing unexpected results.

[raboof]

Running the UriParserBenchmark this does seem to have a somewhat visible impact:

jmh:run -i 3 -wi 3 -f 1 .*UriParserBenchmark

Before:

[info] Iteration   1: 621.705 ops/ms
[info] Iteration   2: 625.823 ops/ms
[info] Iteration   3: 627.470 ops/ms
[info] Iteration   1: 284.254 ops/ms
[info] Iteration   2: 282.141 ops/ms
[info] Iteration   3: 282.596 ops/ms

[info] Iteration   1: 608.121 ops/ms
[info] Iteration   2: 638.491 ops/ms
[info] Iteration   3: 635.331 ops/ms

[info] Iteration   1: 282.200 ops/ms
[info] Iteration   2: 282.684 ops/ms
[info] Iteration   3: 278.946 ops/ms

After:

[info] Iteration   1: 612.108 ops/ms
[info] Iteration   2: 610.006 ops/ms
[info] Iteration   3: 618.760 ops/ms
[info] Iteration   1: 277.478 ops/ms
[info] Iteration   2: 276.727 ops/ms
[info] Iteration   3: 275.033 ops/ms

[info] Iteration   1: 618.215 ops/ms
[info] Iteration   2: 615.976 ops/ms
[info] Iteration   3: 624.117 ops/ms
[info] Iteration   1: 268.670 ops/ms
[info] Iteration   2: 272.804 ops/ms
[info] Iteration   3: 269.275 ops/ms

Now this is of course a bit of a microbenchmarks and as such to be taken with a grain of salt, but as we don't really seem to have a strong use case for this feature perhaps we should not take the hit?

[jrudolph]

Sorry for keeping this around for so long.

I cannot see any significant performance difference. On my machine with JDK 8, I can see an (at least) bimodal distribution of running times when using higher fork values for JMH (as you should always run it with). As so often the Hotspot not always arrives at the same steady JIT-generated code, which seems to make a big difference here. In any case, the difference between before and after seems to be small and not definitely in one direction, so this is fine for me the be included.

[jrudolph]

(I refreshed the original branch)