Jim & Phil, I added the following class header in Byte/Int/Float ArrayCache classes and removed the shell script:
/* * Note that the [BYTE/INT/FLOAT]ArrayCache files are nearly identical except * for a few type and name differences. Typically, the [BYTE]ArrayCache.java file * is edited manually and then [INT]ArrayCache.java and [FLOAT]ArrayCache.java * files are generated with the following command lines: */ // % sed -e 's/(b\yte)[ ]*//g' -e 's/b\yte/int/g' -e 's/B\yte/Int/g' < B\yteArrayCache.java > IntArrayCache.java // % sed -e 's/(b\yte)[ ]*/(float) /g' -e 's/b\yte/float/g' -e 's/B\yte/Float/g' < B\yteArrayCache.java > FloatArrayCache.java I tested the commands and it works well. Here is the updated webrev: http://cr.openjdk.java.net/~lbourges/marlin/marlin-8159638.2/ Cheers, Laurent 2016-08-03 2:28 GMT+02:00 Jim Graham <james.gra...@oracle.com>: > How about instead of the shell script we put a comment up at the top of > the files (after the copyright header), with the appropriate command line? > Something like: > > /* > * Note that the Byte/Int/Float files are nearly identical except > * for a few type and name differences. Typically, the Byte version > * is edited manually and then the Int and Float versions are > * generated with the following command lines: > * % sed ... Float ... > * % sed ... Int ... > */ > > The only issue is trying to word this in a way that prevents the "Byte" in > this comment from being converted. It gets even tricker when we have the > strings being substituted appear in the command lines. Perhaps escapes > would avoid the issue? And upper case? Something like this (but it looks > kind of gross): > > /* > * Note that the BYTE/INT/FLOAT files are nearly identical except > * for a few type and name differences. Typically, the BYTE version > * is edited manually and then the INT and FLOAT versions are > * generated with the following command lines: > * % sed ... \b\y\t\e ... float ... \B\y\t\e ... Float ... > * % sed ... \b\y\t\e ... int ... \B\y\t\e ... Int ... > */ > > A developer could either cut and paste the commands to a command line, or > write their own shell script... > > ...jim > > > On 08/02/2016 03:34 PM, Philip Race wrote: > >> I have not yet looked at everything but no issues except that >> I find checking in the shell script a bit weird. >> Not to mention its technically a "source file" so should have a license. >> >> -phil. >> >> On 8/2/16, 2:56 PM, Jim Graham wrote: >> >>> Thanks Laurent, >>> >>> On 08/02/2016 05:57 AM, Laurent Bourgès wrote: >>> >>>> Thanks for the tip, I made another webrev (for archive) that shows the >>>> proper diffs in ArrayCache / ArrayCacheConst: >>>> http://cr.openjdk.java.net/~lbourges/marlin/marlin-8159638.1_bis/ >>>> <http://cr.openjdk.java.net/%7Elbourges/marlin/marlin-8159638.1_bis/> >>>> >>> >>> Thanks! >>> >>> In Renderer.java, you create the alphaLine and blkFlags refs as >>>> Clean, but then you always put them back using indices of (0, 0) so >>>> they will never actually be cleaned - is there a reason you don't >>>> just use a dirty ref there? >>>> >>>> Both alphaLine and blkFlags arrays must be zero-filled as these arrays >>>> are storing accumulated values: >>>> >>>> It is not possible to use a dirty reference in this case as both >>>> allocated and returned array may contain garbage data (from the >>>> IntArrayCache). >>>> >>> >>> D'oh! I guess that was obvious. I wasn't thinking of the fact that >>> dirty caches can initially return a non-zero-filled array - the fact >>> that they clean on "put" is only half of their zero guarantee... >>> >>> Other than that question, I don't see any problems with the fix... >>>> >>>> Ready to go ? >>>> or I need another reviewer, phil ? >>>> >>> >>> Ready from my end. Phil? >>> >>> ...jim >>> >>> -- -- Laurent Bourgès
