[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-core-libs-dev
Subject:    Review request for #4853493
From:       martinrb () google ! com (Martin Buchholz)
Date:       2010-05-17 23:06:27
Message-ID: AANLkTildiKG-FfOPh183t-DulCeAQGSrYP5AODV12gOt () mail ! gmail ! com
[Download RAW message or body]

Looks good!

Martin

On Mon, May 17, 2010 at 15:59, Xueming Shen <xueming.shen at oracle.com> wrote:
>
> It appears the "inline" version makes the GZIPOutputStream.class about 140
> byte smaller. And I would
> guess it might be also faster. The webrev has been updated to go with the
> "inline" version.
>
> Thanks,
> -Sherman
>
> Martin Buchholz wrote:
>>
>> On Mon, May 17, 2010 at 13:33, Xueming Shen <xueming.shen at oracle.com>
>> wrote:
>>
>>>
>>> Martin,
>>>
>>> It appears we should make the defensive copy in this case, as we usually
>>> do
>>> in lib code.
>>>
>>
>> Yeah, that does look necessary. ?Approved!
>>
>> You could also keep the header code
>> inside the method thus:
>>
>> ? ?private void writeHeader() throws IOException {
>> ? ? ? ?out.write(new byte[] {
>> ? ? ? ? ? ? ? ? ? ? ?(byte) GZIP_MAGIC, ? ? ? // Magic number (short)
>> ? ? ? ? ? ? ? ? ? ? ?(byte)(GZIP_MAGIC >> 8), // Magic number (short)
>> ? ? ? ? ? ? ? ? ? ? ?Deflater.DEFLATED, ? ? ? // Compression method (CM)
>> ? ? ? ? ? ? ? ? ? ? ?0, ? ? ? ? ? ? ? ? ? ? ? // Flags (FLG)
>> ? ? ? ? ? ? ? ? ? ? ?0, ? ? ? ? ? ? ? ? ? ? ? // Modification time MTIME
>> (int)
>> ? ? ? ? ? ? ? ? ? ? ?0, ? ? ? ? ? ? ? ? ? ? ? // Modification time MTIME
>> (int)
>> ? ? ? ? ? ? ? ? ? ? ?0, ? ? ? ? ? ? ? ? ? ? ? // Modification time MTIME
>> (int)
>> ? ? ? ? ? ? ? ? ? ? ?0, ? ? ? ? ? ? ? ? ? ? ? // Modification time MTIME
>> (int)
>> ? ? ? ? ? ? ? ? ? ? ?0, ? ? ? ? ? ? ? ? ? ? ? // Extra flags (XFLG)
>> ? ? ? ? ? ? ? ? ? ? ?0 ? ? ? ? ? ? ? ? ? ? ? ?// Operating system (OS)
>> ? ? ? ? ? ? ? ? ?});
>> ? ?}
>>
>>
>>
>>>
>>> http://cr.openjdk.java.net/~sherman/4853493/webrev
>>>
>>> -Sherman
>>>
>>>
>
>

[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic