[OpenJDK 2D-Dev] [jdk17] Withdrawn: 8271489: (doc) Clarify Filter Factory example

[Roger Riggs]

On Thu, 29 Jul 2021 16:36:21 GMT, Roger Riggs  wrote:

> Improve the clarity of comments in the ObjectInputFilter FilterInThread 
> example.

This pull request has been closed without being integrated.

-

PR: https://git.openjdk.java.net/jdk17/pull/292

[OpenJDK 2D-Dev] [jdk17] RFR: 8271489: (doc) Clarify Filter Factory example

[Roger Riggs]

Improve the clarity of comments in the ObjectInputFilter FilterInThread example.

-

Commit messages:
 - 8271489: (doc) Clarify Filter Factory example
 - 8270398: Enhance canonicalization
 - 8270404: Better canonicalization
 - Merge
 - Merge
 - 8263531: Remove unused buffer int
 - 8262731: [macOS] Exception from "Printable.print" is swallowed during 
"PrinterJob.print"
 - 8269763: The JEditorPane is blank after JDK-8265167
 - 8265580: Enhanced style for RTF kit
 - 8265574: Improve handling of sheets
 - ... and 15 more: 
https://git.openjdk.java.net/jdk17/compare/c1304519...650e1561

Changes: https://git.openjdk.java.net/jdk17/pull/292/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk17=292=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8271489
  Stats: 1001 lines in 42 files changed: 625 ins; 181 del; 195 mod
  Patch: https://git.openjdk.java.net/jdk17/pull/292.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk17 pull/292/head:pull/292

PR: https://git.openjdk.java.net/jdk17/pull/292

Re: [OpenJDK 2D-Dev] 8187898: PrintStream should override FilterOutputStream#write(byte[]) with a method that has no throws clause

[Roger Riggs]

Hi Brian,

Would it be appropriate to add @Override to the new method (and perhaps 
existing overridden methods).


Previously, calling FilterOutputStream.write(byte[]) would delegate to 
write(byte[], 0, length).
The proposed change duplicates the code and changes the ways that 
overridden classes might see the call.

What's the benefit of duplicating the code and calling out.write(buf)?

Not my call, but in PSPrinterJob.java (970-977) it might be cleaner to 
just delete the unused code and comment.


(The CSR will need to note the source incompatibility due to the removal 
of a declared exception)


Thanks, Roger

On 7/11/19 1:44 PM, Brian Burkhalter wrote:

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

Override FilterOutputStream.write(byte[]) not to throw IOException. Including 
2d-dev as this creates a source compatibility issue in PSPrinterJob which I fix 
in this patch.

A CSR would of course be needed for this.

Thanks,

Brian

Re: [OpenJDK 2D-Dev] RFR 8214014 : Remove vestiges of gopher: protocol proxy support

[Roger Riggs]

Hi Phil,

ok, with your explanation, I don't see any use to removing it
or the other similar protocol identifiers that apply to IPP.

Thanks, Roger


On 11/28/2018 03:22 PM, Phil Race wrote:

The presence of this field doesn't do anything to suggest that
the JDK must support gopher protocol. It is an informational
attribute that may be returned by a remote print service to
say what schemes it supports. You can then "pick" one that
works for you to communicate with the remote print service.
Removing it doesn't really bring a big benefit that I see even
though I think the only thing in the world that might notice is JCK.


Also note this class isn't something JDK dreamed up.

This class is the JDK's implementation of the IPP RFC's 
printer-uri-schemes-supported attribute


The standard uri schemes are discussed here

https://tools.ietf.org/html/rfc2911#section-4.1.6

where it also says
 A Printer object MAY support any URI 'scheme' that has been 
registered with IANA [IANA-MT]


I think "IANA-MT" is wrong here .. I think it means this :
https://www.iana.org/assignments/uri-schemes/uri-schemes.xhtml

Quite why some non-standard schemes are listed in this class are
lost in the mists of time but GOPHER is no worse than NEWS, NNTP and 
WAIS in the same class.
Why not remove those too ... the argument is no different , but an 
important
point is the one I made above. This does NOT in any way suggest the 
scheme is supported

by the JDK.

-phil.

On 11/28/18 11:00 AM, Roger Riggs wrote:

Hi Phil,

On 11/28/2018 12:06 PM, Phil Race wrote:
> Please check in javax.print that the inclusion of the gopher 
protocol is no longer needed.


In that case, 2d-dev is the list you want. Swing doesn't do printing.

Thanks for the correction.


It must have been a decade since I heard anyone mention gopher, but did
you really mean to remove a Java SE public API variable ?
Probably yes, but the process needs to be different, to deprecate for 
a release

or two then remove.

Its low cost to keep it but it is noise as far as the rest of the 
codebase goes.
I'll create a separate issue and maybe there are some others that can 
be removed

as well.

Thanks, Roger


  /**
- * Gopher Protocol.
- */
- public static final ReferenceUriSchemesSupported GOPHER = new 
ReferenceUriSchemesSupported(3);

-
- /**

https://docs.oracle.com/en/java/javase/11/docs/api/java.desktop/javax/print/attribute/standard/ReferenceUriSchemesSupported.html#GOPHER

-phil.

On 11/28/18 8:34 AM, Roger Riggs wrote:
Please review the removal of some left over references to the 
gopher protocol related to proxies.

Most are related to properties or setting up proxies (for gopher).

Please check in javax.print that the inclusion of the gopher 
protocol is no longer needed.


Webrev:
http://cr.openjdk.java.net/~rriggs/webrev-gopher-remove-8214014/

CSR:
https://bugs.openjdk.java.net/browse/JDK-8214301

Thanks, Roger

p.s. fyi, this patch assumes a previous patch cleaning up property 
initialization (4947890)

Re: [OpenJDK 2D-Dev] RFR 8214014 : Remove vestiges of gopher: protocol proxy support

[Roger Riggs]

Hi Phil,

On 11/28/2018 12:06 PM, Phil Race wrote:
> Please check in javax.print that the inclusion of the gopher 
protocol is no longer needed.


In that case, 2d-dev is the list you want. Swing doesn't do printing.

Thanks for the correction.


It must have been a decade since I heard anyone mention gopher, but did
you really mean to remove a Java SE public API variable ?
Probably yes, but the process needs to be different, to deprecate for a 
release

or two then remove.

Its low cost to keep it but it is noise as far as the rest of the 
codebase goes.
I'll create a separate issue and maybe there are some others that can be 
removed

as well.

Thanks, Roger


  /**
- * Gopher Protocol.
- */
- public static final ReferenceUriSchemesSupported GOPHER = new 
ReferenceUriSchemesSupported(3);

-
- /**

https://docs.oracle.com/en/java/javase/11/docs/api/java.desktop/javax/print/attribute/standard/ReferenceUriSchemesSupported.html#GOPHER

-phil.

On 11/28/18 8:34 AM, Roger Riggs wrote:
Please review the removal of some left over references to the gopher 
protocol related to proxies.

Most are related to properties or setting up proxies (for gopher).

Please check in javax.print that the inclusion of the gopher protocol 
is no longer needed.


Webrev:
http://cr.openjdk.java.net/~rriggs/webrev-gopher-remove-8214014/

CSR:
https://bugs.openjdk.java.net/browse/JDK-8214301

Thanks, Roger

p.s. fyi, this patch assumes a previous patch cleaning up property 
initialization (4947890)

Re: [OpenJDK 2D-Dev] RFR: 8130264 : change the mechanism by which JDK loads the platform-specific PrinterJob implementation

[Roger Riggs]

Hi Phil,

Looks fine from the core-libs perspective.

Thanks, thanks for removing this cross module dependency.

Roger


On 11/15/2018 04:41 PM, Phil Race wrote:

bug: http://cr.openjdk.java.net/~prr/8130264/
webrev: http://cr.openjdk.java.net/~prr/8130264/

Currently java launcher code embeds the name of the java.desktop 
module's PrinterJob
implementation class for each platform in a system property which is 
later
read by the java.desktop code to use to reflectively locate the class 
and instantiate it.


This fix removes that entirely from the launcher code and the desktop 
module

now looks it up internally via a simple platform proxy class.

This builds on all platforms and we rely on existing printing tests to 
verify

that we can still locate the implementation class.

The new regression test just verifies the system property name space 
is no longer polluted.

I didn't find any test (apart from this new one) that references it.

Since that system property has been around for a long time I am 
thinking I should file a CSR
to document its removal .. unless there is a concensus it is not 
necessary.


-phil.

Re: [OpenJDK 2D-Dev] java.awt.* properties initialization?

[Roger Riggs]

Hi Phil,

Thanks for filing the issues and clarifications.
I agree that removal is a great option to reduce clutter and maintenance.

I forgot to include 'sun.desktop' which seems to be only ever set to 
'gnome' on linux

and 'windows' on windows.

Thanks, Roger



On 10/18/2018 05:39 PM, Phil Race wrote:
Adding 2d-dev .. as most of these are actually for 2D, not AWT, 
despite the names.


I have bugs filed to get rid of these two 2D properties :

https://bugs.openjdk.java.net/browse/JDK-8130264 : java.awt.printerjob
https://bugs.openjdk.java.net/browse/JDK-8130266 : java.awt.graphicsenv
Probably I could file another for "awt.toolkit", which is AWT.

The potential external use of these to inject replacments was made 
moot by jigsaw.


So I don't think I'd bother "moving" them, more like replacing them & 
then deleting this code.


fontpath used to be used as a workaround for failure by 2D to locate 
the system fonts
which probably is no longer needed due to better ways we have of doing 
this these days.

We probably could remove that along with the code which reads it.

I'd have to think about where we could put the isInAquaSession() logic.

-phil.


On 10/18/2018 07:22 AM, Roger Riggs wrote:

Hi,

There are a few java.awt.* properties that have support in the 
system.c native code based on
the operating system.   Most do not need to be in native code and 
decoupling the code

would be a good cleanup.

I'd like to look at the properties and move the initializations to a 
more appropriate place.
Since they are not used in the java.base module it would be more 
appropriate for
them to be handled in the java.desktop module and the appropriate 
package.


At the moment, they all can be overridden by command line arguments 
though

in some cases, that seems to be unintentional and perhaps undesirable.
Most could dispatch on the os_name system property to select the 
appropriate implementation class.


I'd like to work through this issues and see if there are any special 
considerations.

Thanks for any comments and advice.

Roger


java.awt.graphicsenv:  java.awt.GraphicsEnvironment.java

"sun.awt.CGraphicsEnvironment" "sun.awt.X11GraphicsEnvironment" 
"sun.awt.Win32GraphicsEnvironment"


awt.toolkit : java.awt.Toolkit.java

"sun.awt.X11.XToolkit" "sun.lwawt.macosx.LWCToolkit" 
"sun.awt.windows.WToolkit"


java.awt.printerjob: java.awt.print.PrinterJob.java

"sun.print.PSPrinterJob""sun.lwawt.macosx.CPrinterJob" 
"sun.awt.windows.WPrinterJob"


awt.headless:  This may need native code specific to the Mac.

isInAquaSession() ? NULL :"true";


sun.java2d.fontpath: sun.font.SunFontManager
The native initialization is hooked to an environment variable 
"JAVA2D_FONTPATH".

Is that initialization from the environment still used or needed?





--
Thanks, Roger

Re: [OpenJDK 2D-Dev] RFR: 8147443: Use Common Cleaner in Marlin OffHeapArray

[Roger Riggs]

Hi,

This change looks ok to me (from the Cleaner perspective).

(Phil, I'm assuming its just a typo that cleanerRef and cleanerObj 
should be the same.)


Roger


On 1/15/16 6:55 PM, Phil Race wrote:

This looks fine to me. One comment that I am not sure applies here
is that when using the equivalent '2d disposer' mechanism, the object
we pass to the disposer to track when to dispose the resources is kept
as small as possible. For example you have :
Renderer(final RendererContext rdrCtx) {
this.rdrCtx = rdrCtx;

this.edges = new OffHeapArray(rdrCtx, INITIAL_EDGES_CAPACITY); 
// 96K


Now if we add one new field  to RendererContext :
  Object cleanerObj = new Object();

and then change your code to
this.edges = new OffHeapArray(rdrCtx.cleanerRef, 
INITIAL_EDGES_CAPACITY); // 96K


then GC should be able to free up the rest of rdrCtx more promptly.

-phil

On 01/15/2016 02:04 PM, Laurent Bourgès wrote:

Dear all,

Sorry, I need to restart properly the review thread that occured in 
another topic:

http://mail.openjdk.java.net/pipermail/2d-dev/2016-January/006145.html


Please review this webrev refactoring Marlin OffHeapArray to use the 
new jdk.internal.ref.CleanerFactory (new Cleaner API) instead of my 
own solution (ReferenceQueue + PhantomReference + Thread):
http://cr.openjdk.java.net/~lbourges/marlin/marlin-Cleaner.1/ 



bug: https://bugs.openjdk.java.net/browse/JDK-8147443

The new API is really great as it just needs 1 line:
+ // Register a cleaning function to ensure freeing off-heap memory: 
+ CleanerFactory.cleaner().register(parent, () -> this.free());


I added a qualified export in modules.xml:
  
+ jdk.internal.ref
+ java.desktop
+ 
+ 

FYI there is no performance issue as all off-heap arrays are 
belonging to a Marlin's RendererContext (parent reference). The 
RendererContext instances are stored as thread-local variables using 
a SoftReference (or a WeakReference) to maximize the potential reuse 
(like a buffer) but avoid OOME. Finally I tested the patch using the 
following settings to use weak references to RendererContexts and 
enable logging all allocations / free operations in OffHeapArray:

-Dsun.java2d.renderer.log=true
-Dsun.java2d.renderer.logUnsafeMalloc=true
-Dsun.java2d.renderer.useRef=weak

INFO: 1452703097931: OffHeapArray.allocateMemory = 65536 to addr = 
140642864899472
INFO: 1452703097931: OffHeapArray.allocateMemory = 98304 to addr = 
140642864965024

...
INFO: 1452703103321: OffHeapEdgeArray.free = 98304 at addr = 
140642864965024
INFO: 1452703103321: OffHeapEdgeArray.free = 65536 at addr = 
140642864899472

...
INFO: 1452703103827: OffHeapArray.allocateMemory = 65536 to addr = 
140642864910240
INFO: 1452703103827: OffHeapArray.allocateMemory = 98304 to addr = 
140642864975792

...
INFO: 1452703113822: OffHeapEdgeArray.free = 98304 at addr = 
140642864975792
INFO: 1452703113822: OffHeapEdgeArray.free = 65536 at addr = 
140642864910240

...

As mentioned by Phil, other Java2D classes (like Disposer) may use 
the new Cleaner approach in the future and potentially need a 
dedicated Cleaner instance (Thread) for performance sensitive code.


Best regards,
Laurent

Re: [OpenJDK 2D-Dev] Cleaner usage in java2 / awt

[Roger Riggs]

Hi Laurent,

Looks good.

Thanks, Roger


On 1/15/16 4:28 AM, Laurent Bourgès wrote:

Phil & Roger,

Please review the updated webrev according to Roger's comment:
http://cr.openjdk.java.net/~lbourges/marlin/marlin-Cleaner.1/ 
<http://cr.openjdk.java.net/%7Elbourges/marlin/marlin-Cleaner.1/>


Changes:
+ // Register a cleaning function to ensure freeing off-heap memory:
+ CleanerFactory.cleaner().register(parent, () -> this.free());

I created the bug: https://bugs.openjdk.java.net/browse/JDK-8147443
Could you add the proper tags (no-reg, jigsaw ?)

Phil, can I push it myself as modules.xml is modified or you should do 
it for me ?


Regards,
Laurent


2016-01-13 17:56 GMT+01:00 Roger Riggs <roger.ri...@oracle.com 
<mailto:roger.ri...@oracle.com>>:


Hi Laurent,

Nice use.

You don't need to use the local_array in this case.
Since the reference being cleaned is a the parent object it is ok
that the closure captures 'this'.

The modules.xml qualified export is fine.

Roger



On 1/13/16 11:48 AM, Laurent Bourgès wrote:

Phil & Roger,

Please review this first webrev refactoring Marlin OffHeapArray
to use the new jdk.internal.ref.CleanerFactory (new Cleaner API):
http://cr.openjdk.java.net/~lbourges/marlin/marlin-Cleaner.0/
<http://cr.openjdk.java.net/%7Elbourges/marlin/marlin-Cleaner.0/>

The new API is really great as it just needs few lines:
+ // Register a cleaning function to ensure freeing off-heap memory:
+ final OffHeapArray local_array = this; // local to prevent
capture of this
+ CleanerFactory.cleaner().register(parent, () ->
local_array.free());

I added a qualified export in modules.xml:
  
+ jdk.internal.ref
+ java.desktop
+ 
+ 
Who can review such changes to modules.xml (out of my scope) ?

Please check I made it right as I cannot test it with jake !


I tested it using the following settings to use weak references
to Marlin RendererContexts and log all allocations / free buffers
in OffHeapArray:
-Dsun.java2d.renderer.log=true
-Dsun.java2d.renderer.logUnsafeMalloc=true
-Dsun.java2d.renderer.useRef=weak

INFO: 1452703097931: OffHeapArray.allocateMemory = 65536 to addr
= 140642864899472
INFO: 1452703097931: OffHeapArray.allocateMemory = 98304 to addr
= 140642864965024
...
INFO: 1452703103321: OffHeapEdgeArray.free = 98304 at addr =
140642864965024
INFO: 1452703103321: OffHeapEdgeArray.free = 65536 at addr =
140642864899472
...
INFO: 1452703103827: OffHeapArray.allocateMemory = 65536 to addr
= 140642864910240
INFO: 1452703103827: OffHeapArray.allocateMemory = 98304 to addr
= 140642864975792
...
INFO: 1452703113822: OffHeapEdgeArray.free = 98304 at addr =
140642864975792
INFO: 1452703113822: OffHeapEdgeArray.free = 65536 at addr =
140642864910240
...

PS: I did not create a bug yet.

Cheers,
Laurent





--
--
Laurent Bourgès

Re: [OpenJDK 2D-Dev] Cleaner usage in java2 / awt

[Roger Riggs]

Hi Laurent,

Nice use.

You don't need to use the local_array in this case.
Since the reference being cleaned is a the parent object it is ok that 
the closure captures 'this'.


The modules.xml qualified export is fine.

Roger


On 1/13/16 11:48 AM, Laurent Bourgès wrote:

Phil & Roger,

Please review this first webrev refactoring Marlin OffHeapArray to use 
the new jdk.internal.ref.CleanerFactory (new Cleaner API):
http://cr.openjdk.java.net/~lbourges/marlin/marlin-Cleaner.0/ 



The new API is really great as it just needs few lines:
+ // Register a cleaning function to ensure freeing off-heap memory:
+ final OffHeapArray local_array = this; // local to prevent capture 
of this

+ CleanerFactory.cleaner().register(parent, () -> local_array.free());

I added a qualified export in modules.xml:
  
+ jdk.internal.ref
+ java.desktop
+ 
+ 
Who can review such changes to modules.xml (out of my scope) ?

Please check I made it right as I cannot test it with jake !


I tested it using the following settings to use weak references to 
Marlin RendererContexts and log all allocations / free buffers in 
OffHeapArray:

-Dsun.java2d.renderer.log=true
-Dsun.java2d.renderer.logUnsafeMalloc=true
-Dsun.java2d.renderer.useRef=weak

INFO: 1452703097931: OffHeapArray.allocateMemory = 65536 to addr = 
140642864899472
INFO: 1452703097931: OffHeapArray.allocateMemory = 98304 to addr = 
140642864965024

...
INFO: 1452703103321: OffHeapEdgeArray.free = 98304 at addr = 
140642864965024
INFO: 1452703103321: OffHeapEdgeArray.free = 65536 at addr = 
140642864899472

...
INFO: 1452703103827: OffHeapArray.allocateMemory = 65536 to addr = 
140642864910240
INFO: 1452703103827: OffHeapArray.allocateMemory = 98304 to addr = 
140642864975792

...
INFO: 1452703113822: OffHeapEdgeArray.free = 98304 at addr = 
140642864975792
INFO: 1452703113822: OffHeapEdgeArray.free = 65536 at addr = 
140642864910240

...

PS: I did not create a bug yet.

Cheers,
Laurent

Re: [OpenJDK 2D-Dev] Remove redundant package name in src/java.desktop/share/classes/sun/java2d/Disposer.java

[Roger Riggs]

Hi NingHua,

This topic might be most appropriate for the 2d-dev@openjdk.java.net alias

This email strips attachments so an inline patch might be the easiest 
way to propose the change.


Roger


On 4/23/2015 1:19 AM, ning...@linux.vnet.ibm.com wrote:

A. The problem to be resolve or feature to be added
The class name includes package name java.security/java.lang.ref 
though they were already imported


B. The solution proposed by this patch
Remove package name java.security/java.lang.ref from class name

C: Modified file
src/java.desktop/share/classes/sun/java2d/Disposer.java

D:Patch
It is attached as webrev-OJDK-943-OpenJDK9.zip


Regards,

NingHua