[OpenJDK 2D-Dev] [PATCH] FontManager refactoring

[Roman Kennke]

Hi Phil,

> >>  >> I'm not sure about the movement of addToPool() to TrueTypeFont.java
> >>  >> Theoretically its independent of the font format, and could apply
> >>  >> to any FileFont subclass.
> >>  >> also the file closer shut down hook isn't needed except for created
> >>  >> fonts from tmp files, so that shouldn't be started unless needed.
> >>  >
> >>  > I moved this code to a new class called FontChannelPool, and only start
> >>  > the hook when needed.
> >>
> >> I see that in FontChannelPool.java you start the closer when the TT font is a copy.
> >> I don't have your previous webrev any more to compare but isn't
> >> quite what I expected. The "closer" hook is so we can delete
> >> files, and its started from the createFont2D() code, which is
> >> the only case when we'd want to delete the files. You even
> >> left it in there, called fileCloser, but it no longer closes
> >> the files, just deletes them.
> >>
> >> So: there should be no need to add the "isCopy" to TrueTypeFont,
> >> nor to pass a variable to addToPool().
> >>     
> >
> > I removed these isCopy things. Somehow, I tried to be close in behaviour
> > to the original behaviour, but it actually doesn't make any sense (only
> > closing channels if somebody called createFont(InputStream) ? Must be a
> > mistake). I hope that this is what you were critizing, no?
> >
> >   
> 
> I haven't yet looked at the new webrev, but the issue is this :
> 
> Most apps use only "system" installed fonts, or other ones that are 
> permanently on disk.
> There can be a lot of these.
> Closing files in the pool happens routinely as we limit the number that 
> are open at one time.
> 
> When the VM exits normally (ie via System.exit(int)), then the 
> underlying operating system
> will close the file descriptors for us and we have no clean up (*)  we 
> need to do, so there
> is no need to explicitly close these files.
> 
>  However in the case of fonts created via Font.createFont(InputStream) 
> the implementation
> always copies this data into a temporary disk file before doing 
> anything. So when the
> JRE exits we do need to do some clean up. ie delete the temp files from 
> the temp dir.
> On UFS based systems, and likely all filesystems on Unix systems, this means
> calling the system "unlink" (which is what File.delete does) and it 
> matters not one whit
> whether rhe file is open at the time we call unlink(). The O/S and 
> filesystem more or
> less immediately hides it from new readers of the containing directory. When
> the last process already using that file exits, the file is "gone" - 
> deleted.
> 
> But on MS-Windows, if any process has the file open, then File.delete 
> simply fails
> So in order to delete these files, they need to be first closed.
> 
> Hopefully you can see this applies *only* to fonts created by a 
> Font.createFont(InputStream)


Ok, this makes sense now :-) Now I'm thinking that it's probably best to
go back to the original implementation, except that it's in
SunFontManager instead of FontManager. Do you agree?

Cheers, Roman

-- 
Dipl.-Inform. (FH) Roman Kennke, Software Engineer, http://kennke.org
aicas Allerton Interworks Computer Automated Systems GmbH
Haid-und-Neu-Straße 18 * D-76131 Karlsruhe * Germany
http://www.aicas.com   * Tel: +49-721-663 968-48
USt-Id: DE216375633, Handelsregister HRB 109481, AG Karlsruhe
Geschäftsführer: Dr. James J. Hunt