Re: [10] RFR: 8186930: Constant fold URI constants
One way to proceed in the future is to use those static methods as constant producing functions (via condy), then at jlink time run the functions and possibly strip out the then redundant code. Paul. > On 30 Aug 2017, at 09:16, Daniel Fuchs wrote: > > Hi Claes, > > Maybe it could be interesting to leave the > methods in a static inner class which is > not referenced (except by whitebox tests) and > have this inner class expose a static test() > method that would test that the static fields > in URI have the expected value? > > This is just a suggestion, and it > could be done in a followup RFE... > > best regards, > > -- daniel > > On 31/08/2017 16:26, Claes Redestad wrote: >> On 2017-08-30 17:17, Alan Bateman wrote: >>> I think it could be useful as someone reading the code isn't going to >>> immediately know to jump to URI. >> Done: http://cr.openjdk.java.net/~redestad/8186930/jdk.01/ >> /Claes >
Re: [10] RFR: 8186930: Constant fold URI constants
On 2017-08-30 18:26, Alan Bateman wrote: This looks good to me. Thanks, Alan! /Claes
Re: [10] RFR: 8186930: Constant fold URI constants
On 2017-08-30 18:18, Chris Hegarty wrote: Thanks for doing this Claes, this latest version looks good to me. Thanks Chris! /Claes
Re: [10] RFR: 8186930: Constant fold URI constants
Hi Daniel, I'm not sure: I have no love for whitebox tests in general, and especially not for code where doing exhaustive black box tests against a public API is possible and perhaps even straightforward (the test coverage seems adequate currently, but can of course always be improved). /Claes On 2017-08-30 18:16, Daniel Fuchs wrote: Hi Claes, Maybe it could be interesting to leave the methods in a static inner class which is not referenced (except by whitebox tests) and have this inner class expose a static test() method that would test that the static fields in URI have the expected value? This is just a suggestion, and it could be done in a followup RFE... best regards, -- daniel On 31/08/2017 16:26, Claes Redestad wrote: On 2017-08-30 17:17, Alan Bateman wrote: I think it could be useful as someone reading the code isn't going to immediately know to jump to URI. Done: http://cr.openjdk.java.net/~redestad/8186930/jdk.01/ /Claes
Re: [10] RFR: 8186930: Constant fold URI constants
On 31/08/2017 16:26, Claes Redestad wrote: On 2017-08-30 17:17, Alan Bateman wrote: I think it could be useful as someone reading the code isn't going to immediately know to jump to URI. Done: http://cr.openjdk.java.net/~redestad/8186930/jdk.01/ This looks good to me. -Alan
Re: [10] RFR: 8186930: Constant fold URI constants
> On 31 Aug 2017, at 16:26, Claes Redestad wrote: > > > On 2017-08-30 17:17, Alan Bateman wrote: >> I think it could be useful as someone reading the code isn't going to >> immediately know to jump to URI. > > Done: http://cr.openjdk.java.net/~redestad/8186930/jdk.01/ Thanks for doing this Claes, this latest version looks good to me. -Chris.
Re: [10] RFR: 8186930: Constant fold URI constants
Hi Claes, Maybe it could be interesting to leave the methods in a static inner class which is not referenced (except by whitebox tests) and have this inner class expose a static test() method that would test that the static fields in URI have the expected value? This is just a suggestion, and it could be done in a followup RFE... best regards, -- daniel On 31/08/2017 16:26, Claes Redestad wrote: On 2017-08-30 17:17, Alan Bateman wrote: I think it could be useful as someone reading the code isn't going to immediately know to jump to URI. Done: http://cr.openjdk.java.net/~redestad/8186930/jdk.01/ /Claes
Re: [10] RFR: 8186930: Constant fold URI constants
On 2017-08-30 17:17, Alan Bateman wrote: I think it could be useful as someone reading the code isn't going to immediately know to jump to URI. Done: http://cr.openjdk.java.net/~redestad/8186930/jdk.01/ /Claes
Re: [10] RFR: 8186930: Constant fold URI constants
On 31/08/2017 00:11, Claes Redestad wrote: : I just wonder if ParseUtil should keep the lowMask/highMask methods in comments so that further maintainers can satisfy themselves that the values are correct. I thought leaving a comment in ParseUtil that they can be still found in URI was sufficient (cuts back on the redundancy), but I can keep them in comments in both places if you think that's preferable. I think it could be useful as someone reading the code isn't going to immediately know to jump to URI. -Alan
Re: [10] RFR: 8186930: Constant fold URI constants
On 2017-08-30 16:10, Alan Bateman wrote: On 29/08/2017 21:26, Claes Redestad wrote: Webrev: http://cr.openjdk.java.net/~redestad/8186930/jdk.00/ This looks good to me. Thanks! I just wonder if ParseUtil should keep the lowMask/highMask methods in comments so that further maintainers can satisfy themselves that the values are correct. I thought leaving a comment in ParseUtil that they can be still found in URI was sufficient (cuts back on the redundancy), but I can keep them in comments in both places if you think that's preferable. /Claes
Re: [10] RFR: 8186930: Constant fold URI constants
On 29/08/2017 21:26, Claes Redestad wrote: Hi, please review this patch to pre-calculate constants in java.net.URI and sun.net.www.ParseUtil, removing work from runtime (reduces bytecodes executed in the interpreter on bootstrap by ~15K). This also removes use of BitSet from ParseUtil, which apart from being a startup improvement also yields a small throughput improvement (~5%). Webrev: http://cr.openjdk.java.net/~redestad/8186930/jdk.00/ This looks good to me. I just wonder if ParseUtil should keep the lowMask/highMask methods in comments so that further maintainers can satisfy themselves that the values are correct. -Alan
