[ 
https://issues.apache.org/jira/browse/ACCUMULO-4153?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15165688#comment-15165688
 ] 

ASF GitHub Bot commented on ACCUMULO-4153:
------------------------------------------

Github user wjsl commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/73#discussion_r54022815
  
    --- Diff: 
core/src/main/java/org/apache/accumulo/core/file/rfile/bcfile/Compression.java 
---
    @@ -147,25 +148,27 @@ public synchronized OutputStream 
createCompressionStream(OutputStream downStream
         },
     
         GZ(COMPRESSION_GZ) {
    -      private transient DefaultCodec codec;
    +      private transient AtomicReference<DefaultCodec> codec = new 
AtomicReference<DefaultCodec>();
     
           @Override
    -      synchronized CompressionCodec getCodec() {
    -        if (codec == null) {
    -          codec = new DefaultCodec();
    -          codec.setConf(conf);
    +      CompressionCodec getCodec() {
    +        DefaultCodec resultCodec = codec.get();
    +        if (null == resultCodec) {
    +          DefaultCodec newCodec = new DefaultCodec();
    --- End diff --
    
    Why not just share a singleton codec instance for each kind of codec? 
Looking at the implementations (Default, Gzip, Bzip2), they're all effectively 
stateless (sans the Configuration object you can set on them) because they're 
factories for streams. 


> Change getCodec method in BCFile's Compression class
> ----------------------------------------------------
>
>                 Key: ACCUMULO-4153
>                 URL: https://issues.apache.org/jira/browse/ACCUMULO-4153
>             Project: Accumulo
>          Issue Type: Improvement
>          Components: core
>    Affects Versions: 1.6.5
>            Reporter: marco polo
>            Assignee: marco polo
>            Priority: Minor
>             Fix For: 1.6.6, 1.7.2, 1.8.0
>
>         Attachments: patchCompression
>
>
> Seeing major blockages blockages on the Synchronized method of getCodec.  My 
> initial change was to change it to double checked locking; however, spoke 
> with [~kturner] and I'm changing this to an AtomicReference. Have a patch for 
> the former but am in the process of testing the AtomicReference patch
> Note this is for Gzip



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to