Space before comma

2015-09-14 Thread Ahmed Ashour
Hi all,
I would like to prepare a patch for removing space before comma in .java files 
of 'jdk9/dev/jdk'.  Of course, there are places where spaces are needed for 
vertical alignment with previous/next lines.
Would that add a value, or there is no need for such a patch?
P.S. I signed the OCA
Thanks,Ahmed


Re: Suggested fix for JDK-4724038 (Add unmap method to MappedByteBuffer)

2015-09-14 Thread Robert Muir
On Wed, Sep 9, 2015 at 11:46 AM, Peter Levart  wrote:
>
> By wanting to truly release the resources you allocated, you are essentially
> wanting to manage the resources yourself. If you are willing to track the
> active mapped byte buffers manually yourself, then what about the following
> idea:
>

As Uwe mentioned that is probably not truly necessary. If lucene
cannot delete a file, it retries it later periodically until it works.
So if things were unmapped "soonish", for the lucene case things would
be fine I think.

I do realize other apps may not have that infrastructure/luxury...


Re: Suggested fix for JDK-4724038 (Add unmap method to MappedByteBuffer)

2015-09-14 Thread Mark Miller
It seems less than ideal to count on System.gc to do this as a library
though.

Now the user has to worry about what affect System.gc has on what JVM with
what Garbage Collector and whether or not ExplicitGCInvokesConcurrent was
turned on for the JVM, or...

- Mark

On Wed, Sep 9, 2015 at 11:46 AM Peter Levart  wrote:

>
>
> On 09/09/2015 04:56 PM, Dawid Weiss wrote:
> >> I think it would be best to leave to the application to decide and
> >> implement the tracking and also triggering GC at times when it
> approaches
> >> the limit.
> > I disagree. The GC -- when and how it is triggered -- should be
> > transparent to the application. We don't want to manage GC, we want to
> > (truly) release the resources we allocated (and we know when they are
> > no longer needed).
> >
> > What you suggest is essentially managing GC from application level. I
> > don't think it's the right approach to solve the problem.
> >
> > Dawid
>
> Hi Dawid,
>
> By wanting to truly release the resources you allocated, you are
> essentially wanting to manage the resources yourself. If you are willing
> to track the active mapped byte buffers manually yourself, then what
> about the following idea:
>
> - you track the number of mapped buffers (or mapped address space) that
> you "know" is active in the application manually.
> - you track the number of mapped buffers (or mapped address space) that
> is actually mapped at a particular time (by utilizing an after-unmap
> call-back that would have to be added to MappedByteBuffer API)
> - when the difference of those two tracked quantities reaches certain
> amount or percentage, you give a kick to GC to do it's job, as it is
> lagging behind.
>
> I would not call this managing GC, but just hinting GC at the right
> time. The most burden in this approach would be the manual tracking of
> active buffers, but you are willing to do that anyway by wanting to
> manually release the resources. Everything else can be made automatic.
>
>
> Regards, Peter
>
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
> For additional commands, e-mail: dev-h...@lucene.apache.org
>
> --
- Mark
about.me/markrmiller


Fwd: Question about CachedRowSetImpl

2015-09-14 Thread Carolyn Kim
Hi,

I have a question about CachedRowSetImpl. I'm trying to use its pagination
function. But it seems like it does not work with populate method. I think
I'm seeing the same issue that's described here:
https://bugs.openjdk.java.net/browse/JDK-6382534?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel

I am currently using JDK 7.55 and I'm still seeing this issue. Could you
tell me which version of JDK includes this fix?

Thank you very much,
Carolyn


Re: RFR: 8129957 - Deadlock in JNDI LDAP implementation when closing the LDAP context

2015-09-14 Thread Rob McKenna

Hi folks,

So on further investigation it looks like we could get away with 
reducing the amount of locking in LdapClient. Here is a proposed fix 
followed by a description:


http://cr.openjdk.java.net/~robm/8129957/webrev.02/

processConnectionClosure():
- Remove the synchronization from processConnectionClosure and handle it 
further down in notifyUnsolicited


removeUnsolicited():
- Remove the synchronized block in removeUnsolicited as its redundant. 
Vectors are synchronized already.


processUnsolicited()
- Remove the initial synchronized block in processUnsolicited and limit 
it to the area around the unsolicited size check / notice creation. 
(again, due to the notifyUnsolicited changes)
- Remove the redundant unsolicited.size check from the end of 
processUnsolicited


notifyUnsolicited():
- Synchronize on the unsolicited vector in order to create a copy of 
that vector and empty it if e is a NamingException.
- Outside the notifyUnsolicited synchronize block, loop through the copy 
of unsolicited and call fireUnsolicited on each element.
- The main potential problem with this fix would be if an LdapCtx became 
unsolicited before we got to it in the for loop. However since both 
LdapCtx.fireUnsolicited and LdapCtx.removeUnsolicited sync on 
eventSupport and LdapCtx.fireUnsolicited double checks to make sure it 
is still unsolicited, that should be fine.


-Rob


On 10/08/15 14:06, Rob McKenna wrote:

Hi folks,

We have a hang between LdapClient / Ctx due to the fact that
Connection.cleanup() calls LdapClient.processConnectionClosure which
locks the unsolicited vector and in turn calls LdapCtx.fireUnsolicited
which locks the eventSupport object. Unfortunately when an
LdapCtx.close() occurs at the same time, its removeUnsolicited method
locks the eventSupport object first and then attempts to call
LdapClient.removeUnsolicited which attempts to lock the unsolicited vector.

So:

thread 1:

Connection.cleanup ->
LdapClient.processConnectionClosure (LOCK VECTOR) ->
LdapCtx.fireUnsolicited (LOCK EVENTSUPPORT)

(LdapClient is looping through LdapCtx objects in the unsolicited vector)

thread 2:

LdapCtx.close (LOCK LDAPCTX) ->
LdapCtx.removeUnsolicited (LOCK EVENTSUPPORT) ->
LdapClient.removeUnsolicited (LOCK VECTOR)

(A single LdapCtx removes itself from its LdapClient unsolicited list)


My proposed solution is to have both threads lock the LdapClient before
locking either the unsolicited vector or the eventSupport object.

Webrev at: http://cr.openjdk.java.net/~robm/8129957/webrev.01/

 -Rob


Re: RFR: 8033661: readConfiguration does not cleanly reinitialize the logging system

2015-09-14 Thread Daniel Fuchs

Hi Mandy,

On 13/09/15 00:44, Mandy Chung wrote:

Have you considered keeping the same method name, readConfiguration
with the new remapper parameter?  The downside is the difference that
the reset is not invoked.  It might not matter because as you add in
@apiNote that the existing readConfiguration method may be overridden
for custom LogManager but discouraged to be used by applications.


I have reworked the API notes...

The class level documentation says:

 * 
 * If the "java.util.logging.config.class" property is set, then the
 * property value is treated as a class name.  The given class will be
 * loaded, an object will be instantiated, and that object's constructor
 * is responsible for reading in the initial configuration.  (That object
 * may use other system properties to control its configuration.)  The
 * alternate configuration class can use {@code 
readConfiguration(InputStream)}

 * to define properties in the LogManager.


So with that in mind, I have slightly altered the @apiNotes:

in readConfiguration():

 * @apiNote {@code readConfiguration} is principally useful for
 *establishing the LogManager primordial configuration.
 *
 *Calling this method directly from the application code after the
 *LogManager has been initialized is discouraged.

  etc...

The rationale is that an application which needs to establish
a custom configuration should probably use the
{@code "java.util.logging.config.class"} property, and
call LogManager.readConfiguration(InputStream) from there,
as hinted in the class-level documentation.

in readConfiguration(InputStream):

 * @apiNote {@code readConfiguration} is principally useful for 
applications

 *which use the {@code "java.util.logging.config.class"} property
 *to establish their own custom configuration.
 *
 *Calling this method directly from the application code after the
 *LogManager has been initialized is discouraged.

Same rationale than above. After thinking it over I'm not
sure overriding readConfiguration is something we should
encourage. I would prefer some wording that does not imply
it :-)



1749  * Updates an existing configuration.

We should specify what “existing configuration” is.
If “java.util.logging.config.class” is set and it’s instantiated successfully,
readConfiguration will not read from any configuration file.
updateConfiguration only reads the config file regardless
if “java.util.logging.config.class” is set.


I updated the doc for updateConfiguration(mapper):

 * Updates an existing configuration.
 * 
 * @implSpec
 * This is equivalent to calling:
 * 
 *   try (final InputStream in = new 
FileInputStream(logging.properties)) {

 *   final BufferedInputStream bin = new BufferedInputStream(in);
 *   updateConfiguration(bin, mapper);
 *   }
 * 
 * where {@code } is the logging configuration 
file path.

 * 
 * Note that this method not take into account the value of the
 * {@code "java.util.logging.config.class"} property.
 * The {@code "java.util.logging.config.file"} system property is read
 * to find the logging properties file, whether the
 * {@code "java.util.logging.config.class"} property is set or not.
 * If the {@code "java.util.logging.config.file"} system property 
is not

 * defined then the default configuration is used.
 * The default configuration is typically loaded from the 
properties file
 * "{@code conf/logging.properties}" in the Java installation 
directory.

 *

in updateConfiguration(InputStream, mapper), after the table:

 * 
 * Note that this method has no special handling for the "config" 
property.

 * The new value provided by the mapper function will be stored in the
 * LogManager properties, but {@code updateConfiguration} will not 
parse its

 * value nor instantiate the classes it points to.




1761  * @param remapper

“re” probably not needed here.   I think “mapper” should work.


OK. Should I still use the term of 'remapping function' then?
Or does that become 'mapping function' too?


Here is the new webrev - with the other comments from your previous
mail also integrated:

http://cr.openjdk.java.net/~dfuchs/webrev_8033661/webrev.01/

best regards,

-- daniel




Re: Space before comma

2015-09-14 Thread Roger Riggs

Hi Ahmed,

Such a changeset doesn't add much value by itself.  Have you looked for 
some small scale
bug that would be interesting?  Try looking for issues with a label like 
jdk-starter. [1]


Pick something that seems obvious and you feel you can completely understand
the issue, how to write a test first to prove its an issue, and then the 
bug fix

that passes the test.

Roger

[1] query like:  labels in (starter, starterbugs, starterbug, 
starter_bugs, starter_bug, starter-bug, starter-bugs, openjdk-starter, 
jdk-starter) AND resolution = unresolved



On 9/14/2015 7:05 AM, Ahmed Ashour wrote:

Hi all,
I would like to prepare a patch for removing space before comma in .java files 
of 'jdk9/dev/jdk'.  Of course, there are places where spaces are needed for 
vertical alignment with previous/next lines.
Would that add a value, or there is no need for such a patch?
P.S. I signed the OCA
Thanks,Ahmed




JDK 9 RFR of JDK-8136506: Include sun.arch.data.model as a property that can be queried by jtreg

2015-09-14 Thread Joseph D. Darcy

Hello,

Please review the patch below for

JDK-8136506: Include sun.arch.data.model as a property that can be 
queried by jtreg


The jtreg TEST.ROOT file in the HotSpot repository puts

sun.arch.data.model

on the list of properties which can be queried by an @requires clause in 
jtreg. This property lets one determine if a jdk/jre is 32-bits or 64-bits.


It would be convenient and consistent for this property to be available 
to be queried by tests in the jdk repo too.


Thanks,

-Joe

diff -r 64827b676968 test/TEST.ROOT
--- a/test/TEST.ROOTMon Sep 14 19:54:58 2015 +0300
+++ b/test/TEST.ROOTMon Sep 14 17:24:52 2015 -0700
@@ -23,5 +23,8 @@
 # Group definitions
 groups=TEST.groups [closed/TEST.groups]

+# Allow querying of sun.arch.data.model in @requires clauses
+requires.properties=sun.arch.data.model
+
 # Tests using jtreg 4.1 b11 features
 requiredVersion=4.1 b11



Re: RFR: 8033661: readConfiguration does not cleanly reinitialize the logging system

2015-09-14 Thread Daniel Fuchs

Hi Mandy,

Thanks a lot for the feedback!

On 13/09/15 00:44, Mandy Chung wrote:



On Sep 9, 2015, at 9:55 AM, Daniel Fuchs  wrote:

Hi,

Please find below a candidate fix for:

8033661: readConfiguration does not cleanly reinitialize the
 logging system
https://bugs.openjdk.java.net/browse/JDK-8033661

http://cr.openjdk.java.net/~dfuchs/webrev_8033661/webrev.00/

LogManager.readConfiguration() presents a number of flaws that may not
be fixable without endangering backward compatibility, or stability.
Some of them have been quite aptly summed up in JDK-8033661.


Thanks for the detailed analysis.  I agree that adding a new method to
reinitialize the logging configuration is the right thing to do.

Have you considered keeping the same method name, readConfiguration with
the new remapper parameter?  The downside is the difference that the
reset is not invoked.


No - I didn't consider it. The algorithm implemented by the new
method is quite different from what was implemented in the
previous readConfiguration() methods - so the idea of overloading
readConfiguration() didn't come to my mind.


 It might not matter because as you add in @apiNote that the existing
 readConfiguration method may be overridden for custom LogManager but
 discouraged to be used by applications.



Related question: should the existing readConfiguration methods be
deprecated and recommend to use the new method(s)?


What I see is that the old readConfiguration does the appropriate job
when called for the first time, before any logger has been created.
There is probably some application out there that override it to
install their own configuration.
For this reason I am not sure if it should be deprecated.


 Can custom LogManager call updateConfiguration in their constructor
 instead of overriding readConfiguration?


I believe that would be bad - it would go against what we've been
trying to do with https://bugs.openjdk.java.net/browse/JDK-8023168



If the existing readConfiguration methods should be deprecated,
i think keeping the same method name may be a better choice.


I'll leave the decision for that (keeping the same name) to you.
I'm not sure we can deprecate the old methods.
IMHO it is difficult to deprecate a method that is actually
called internally by LogManager, and stop calling it would be
risky WRT to compatibility.




1749  * Updates an existing configuration.

We should specify what “existing configuration” is.
If “java.util.logging.config.class” is set and it’s instantiated successfully,
readConfiguration will not read from any configuration file.
updateConfiguration only reads the config file regardless if
“java.util.logging.config.class” is set.


Right. That's a good point.


1761  * @param remapper

“re” probably not needed here.   I think “mapper” should work.


OK



Use @throws instead of @exception


OK



VisitedLogger class:
1714 public static VisitedLoggers of(Map visited) {
1715 assert visited == null || visited instanceof IdentityHashMap;
1716 return visited == null ? NEVER : new VisitedLoggers(visited);
1717 }

Instead of doing the assert, what about changing the parameter type to 
IdentityHashMap?


can do.



Is VisitedLoggers class necessary?  Would it be more explicit to readers to use 
a Set (it doesn’t need to be a map) that each logger is visited once?


We don't have an IdentityHashSet - which is the reason while I'm using
an IdentityHashMap here.


ConfigurationProperties class: This enum class is also a Predicate and
it has several static methods for updateConfiguratoin to use e.g.

final Stream allKeys = updatePropertyNames.stream()
 .filter(ConfigurationProperties::isOf)
 .filter(k -> ConfigurationProperties.isChanging(k, previous, next));

I have to read the comment and follow the implementation of these methods to 
figure
out what it’s going on here.  It could be simplified and made the code easily
to tell what are being filtered here.



It seems to me that you want to define LevelProperty, HandlerProperty, 
UseParentProperty types,
each of which will handle any change (as it’s currently done in the switch 
statement).


yes - which is why the enum is useful - as it allows to model all
these.



ConfigurationProperties.ALL - why not use ConfigurationProperties.values()?


Well - I could reverse the question :-)

best regards,

-- daniel




Mandy





Re: Collections.emptyList().spliterator() is not ORDERED

2015-09-14 Thread Paul Sandoz
On 7 Sep 2015, at 15:23, Paul Sandoz  wrote:
>> By the way, probably it's reasonable then for Arrays.asList to check
>> the array length like:
>> 
>>   public static  List asList(T... a) {
>>   if(a.length == 0)
>> return Collections.emptyList();
>>   return new ArrayList<>(a);
>>   }
>> 
>> This would make Arrays.asList() (without arguments) and
>> Collections.emptyList() perfectly consistent (now their spliterators
>> report different characteristics) and reduce the number of heap
>> objects. Probably there are some caveats I'm not aware of. Sorry if it
>> was already discussed.
>> 
> 
> This has not been discussed, it’s an edge case micro-optimisation but seems 
> reasonable.
> 

There is a reason not to do this. At the moment Arrays.asList specifies no 
constraints on the identity of the returned List. Adding the micro-optimisation 
will change that. It’s an edge case and a questionable use-case too, but 
considering that i would conservatively leave things as they are.

Paul.