Re: RFR 8198997: Cache normalized/resolved user.dir property

[Brian Burkhalter]

On Mar 21, 2018, at 11:36 AM, Alan Bateman  wrote:

> That looks right.  I assume there's no test for this as it's never been 
> supported to change user.dir on the command line.

I don’t think a test is called for.

Thanks,

Brian

Re: RFR 8198997: Cache normalized/resolved user.dir property

[Alan Bateman]

On 21/03/2018 18:12, Brian Burkhalter wrote:


That was unclear, at least to me:

--- a/src/java.base/windows/classes/java/io/WinNTFileSystem.java
+++ b/src/java.base/windows/classes/java/io/WinNTFileSystem.java
@@ -48,11 +48,11 @@
  public WinNTFileSystem() {
      Properties props = GetPropertyAction.privilegedGetProperties();
      slash = props.getProperty("file.separator").charAt(0);
      semicolon = props.getProperty("path.separator").charAt(0);
      altSlash = (this.slash == '\\') ? '/' : '\\';
-       userDir = props.getProperty("user.dir");
+       userDir = normalize(props.getProperty("user.dir"));
  }

That looks right.  I assume there's no test for this as it's never been 
supported to change user.dir on the command line.


-Alan

Re: RFR 8198997: Cache normalized/resolved user.dir property

[Brian Burkhalter]

On Mar 21, 2018, at 11:08 AM, Alan Bateman  wrote:

> On 21/03/2018 18:02, Brian Burkhalter wrote:
>> Ping …
> I think we're waiting for a new version that will normalize the value of 
> user.dir in the WinNTFileSystem constructor.

That was unclear, at least to me:

--- a/src/java.base/windows/classes/java/io/WinNTFileSystem.java
+++ b/src/java.base/windows/classes/java/io/WinNTFileSystem.java
@@ -48,11 +48,11 @@
 public WinNTFileSystem() {
 Properties props = GetPropertyAction.privilegedGetProperties();
 slash = props.getProperty("file.separator").charAt(0);
 semicolon = props.getProperty("path.separator").charAt(0);
 altSlash = (this.slash == '\\') ? '/' : '\\';
-userDir = props.getProperty("user.dir");
+userDir = normalize(props.getProperty("user.dir"));
 }

Thanks,

Brian

Re: RFR 8198997: Cache normalized/resolved user.dir property

[Alan Bateman]

On 21/03/2018 18:02, Brian Burkhalter wrote:

Ping …
I think we're waiting for a new version that will normalize the value of 
user.dir in the WinNTFileSystem constructor.


-Alan

Re: RFR 8198997: Cache normalized/resolved user.dir property

[Brian Burkhalter]

Ping …

On Mar 13, 2018, at 12:33 PM, Brian Burkhalter  
wrote:

> On Mar 13, 2018, at 12:24 PM, Alan Bateman  wrote:
> 
>> As relative paths are command then it might be simpler to just change the 
>> static initializer
> 
> Static initializer or constructor?
> 
>> to initialize userDir to normalize(props.getProperty("user.dir")).
> 
> I was originally going to put that in the constructor but did not know about 
> calling the normalize() instance method from the ctor.

Re: RFR 8198997: Cache normalized/resolved user.dir property

[Brian Burkhalter]

On Mar 13, 2018, at 12:24 PM, Alan Bateman  wrote:

> As relative paths are command then it might be simpler to just change the 
> static initializer

Static initializer or constructor?

> to initialize userDir to normalize(props.getProperty("user.dir")).

I was originally going to put that in the constructor but did not know about 
calling the normalize() instance method from the ctor.

Brian

Re: RFR 8198997: Cache normalized/resolved user.dir property

[Alan Bateman]

On 12/03/2018 18:32, Brian Burkhalter wrote:

https://bugs.openjdk.java.net/browse/JDK-8198997
http://cr.openjdk.java.net/~bpb/8198997/webrev.00/

Lazily cache the value of the user.dir property on first access. This change is 
for Windows only as there does not appear to be any percentage in doing 
something similar on Unix.

The user.dir property comes from GetCurrentDirectoryW so it will be 
normalized anyway. Although unsupported, I guess there may be some 
deployments setting this property on the command line so it is safer to 
normalize. As relative paths are command then it might be simpler to 
just change the static initializer to initialize userDir to 
normalize(props.getProperty("user.dir")).


-Alan

Re: RFR 8198997: Cache normalized/resolved user.dir property

[Paul Sandoz]

> On Mar 12, 2018, at 11:42 AM, Claes Redestad  
> wrote:
> 
> Hi,
> 
> mixing volatiles and non-volatiles fields that are read and written to 
> outside of
> synchronized makes me somewhat queasy…
> 

Synchronizing on a string also makes me queasy, especially given that string 
can be input by the user, i dunno if there is any interning going on…

It's a little dirty but you might be able to piggyback off another field e.g. 
those of ExpiringCache.

Paul.

> Instead of a volatile boolean and a mutable userDir field, couldn't this just
> as well be modelled as:
> 
> - keep userDir final (always the not normalized value of 
> System.getProperty("user.dir"))
> - replace volatile boolean isUserDirNormal with volatile String 
> normalizedUserDir
> - then implement the DCL in getUserPath like:
> 
> String normalizedUserDir = this.normalizedUserDir;
> if (normalizedUserDir == null) {
> synchronized(userDir) {
> if (normalizedUserDir == null) {
> normalizedUserDir = this.normalizedUserDir = normalize(userDir);
> }
> }
> }
> return normalizedUserDir;
> 
> WDYT?
> 
> /Claes
> 
> On 2018-03-12 19:32, Brian Burkhalter wrote:
>> https://bugs.openjdk.java.net/browse/JDK-8198997
>> http://cr.openjdk.java.net/~bpb/8198997/webrev.00/
>> 
>> Lazily cache the value of the user.dir property on first access. This change 
>> is for Windows only as there does not appear to be any percentage in doing 
>> something similar on Unix.
>> 
>> Thanks,
>> 
>> Brian
> 

Re: RFR 8198997: Cache normalized/resolved user.dir property

[Brian Burkhalter]

Hi Clase,

Thanks for the helpful observations. Given this information I would be inclined 
to agree with your suggested approach.

Brian

On Mar 12, 2018, at 11:53 AM, Claes Redestad  wrote:

> I expect the original user.dir property to be kept resident in memory by 
> System.properties
> (thus no memory loss), and going from a boolean field to an Object reference 
> field on an
> object that I expect to be a singleton(?) is unlikely to be a footprint 
> concern[1].
> 
> /Claes
> 
> [1] Chances are the two approaches are footprint neutral on most archs due to 
> padding.

Re: RFR 8198997: Cache normalized/resolved user.dir property

[Claes Redestad]

Hi Brian,

I expect the original user.dir property to be kept resident in memory by 
System.properties
(thus no memory loss), and going from a boolean field to an Object 
reference field on an
object that I expect to be a singleton(?) is unlikely to be a footprint 
concern[1].


/Claes

[1] Chances are the two approaches are footprint neutral on most archs 
due to padding.



On 2018-03-12 19:46, Brian Burkhalter wrote:

Hi Claes,

That was what I had originally but did not post as I did not like the 
extra String variable increasing the memory footprint. Of course this 
would only be true were the user.dir accessed at all.


Thanks,

Brian

On Mar 12, 2018, at 11:42 AM, Claes Redestad 
> wrote:


Instead of a volatile boolean and a mutable userDir field, couldn't 
this just

as well be modelled as:

- keep userDir final (always the not normalized value of 
System.getProperty("user.dir"))
- replace volatile boolean isUserDirNormal with volatile String 
normalizedUserDir

- then implement the DCL in getUserPath like:

String normalizedUserDir = this.normalizedUserDir;
if (normalizedUserDir == null) {
synchronized(userDir) {
    if (normalizedUserDir == null) {
normalizedUserDir = this.normalizedUserDir = normalize(userDir);
    }
    }
}
return normalizedUserDir;

Re: RFR 8198997: Cache normalized/resolved user.dir property

[Brian Burkhalter]

Hi Claes,

That was what I had originally but did not post as I did not like the extra 
String variable increasing the memory footprint. Of course this would only be 
true were the user.dir accessed at all.

Thanks,

Brian

On Mar 12, 2018, at 11:42 AM, Claes Redestad  wrote:

> Instead of a volatile boolean and a mutable userDir field, couldn't this just
> as well be modelled as:
> 
> - keep userDir final (always the not normalized value of 
> System.getProperty("user.dir"))
> - replace volatile boolean isUserDirNormal with volatile String 
> normalizedUserDir
> - then implement the DCL in getUserPath like:
> 
> String normalizedUserDir = this.normalizedUserDir;
> if (normalizedUserDir == null) {
> synchronized(userDir) {
> if (normalizedUserDir == null) {
> normalizedUserDir = this.normalizedUserDir = normalize(userDir);
> }
> }
> }
> return normalizedUserDir;

Re: RFR 8198997: Cache normalized/resolved user.dir property

[Claes Redestad]

Hi,

mixing volatiles and non-volatiles fields that are read and written to 
outside of

synchronized makes me somewhat queasy...

Instead of a volatile boolean and a mutable userDir field, couldn't this 
just

as well be modelled as:

- keep userDir final (always the not normalized value of 
System.getProperty("user.dir"))
- replace volatile boolean isUserDirNormal with volatile String 
normalizedUserDir

- then implement the DCL in getUserPath like:

String normalizedUserDir = this.normalizedUserDir;
if (normalizedUserDir == null) {
    synchronized(userDir) {
    if (normalizedUserDir == null) {
    normalizedUserDir = this.normalizedUserDir = 
normalize(userDir);

    }
    }
}
return normalizedUserDir;

WDYT?

/Claes

On 2018-03-12 19:32, Brian Burkhalter wrote:

https://bugs.openjdk.java.net/browse/JDK-8198997
http://cr.openjdk.java.net/~bpb/8198997/webrev.00/

Lazily cache the value of the user.dir property on first access. This change is 
for Windows only as there does not appear to be any percentage in doing 
something similar on Unix.

Thanks,

Brian

RFR 8198997: Cache normalized/resolved user.dir property

[Brian Burkhalter]

https://bugs.openjdk.java.net/browse/JDK-8198997
http://cr.openjdk.java.net/~bpb/8198997/webrev.00/

Lazily cache the value of the user.dir property on first access. This change is 
for Windows only as there does not appear to be any percentage in doing 
something similar on Unix.

Thanks,

Brian