#603 Making the PubchemFingerprinter thread safe.

Needs_Revision
closed
nobody
None
master
1
2013-09-24
2013-01-04
No

The PubchemFingerprinter can be made thread safe by moving the SMARTSQueryTool variable to the CountSubstructures class. I think the slight overhead introduced is acceptable given the speedup.

1 Attachments

Discussion

  • John May

    John May - 2013-01-06

    Nope :-). It uses a field to store each fingerprint 'private byte[] m_bits;'. It may not produce any errors but the fingerprinter has a state and thus cannot be thread safe without major refactoring.

     
  • John May

    John May - 2013-01-06

    I think it would be better just to create a wrapper, non? Although it seems a pain the cost of creating a new fingerprinter instance for each molecule is insignificant compared to the actual cost of doing the substructure/ring/path searches. As I understand the PubChem fingerprinter is designed for comprehensiveness and hence is really... really... really.. slow so it definitely won't matter.

    Time (first 50 structures from ChEBI |V| < 50, 2G Ram)

    single instance
    7699
    5797
    5344
    5305
    5450
    5295
    5387
    5282
    5177
    5322
    5462
    5207
    5228

    new instance
    7813
    5619
    5512
    5432
    5273
    5219
    5340
    5294
    5268
    5336

    ... also reminds me we really should upgrade the SMARTS tool to use Asad's SMSD VF2 implementation this would be a lot quicker then.

    Wrapper would look something like this.

    public class ThreadSafePubChemFingerprinter implements IFingerprinter {
    
        @Override
        public IBitFingerprint getBitFingerprint(IAtomContainer container) throws
                                                                           CDKException {
            return new PubchemFingerprinter().getBitFingerprint(container);
        }
    
        // ... other methods just throw and unsupported operation exception
    
    }
    
     
    Last edit: John May 2013-01-06
  • Egon Willighagen

    • Group: Needs_Review --> Needs_Revision
     
  • John May

    John May - 2013-09-23

    Just spoke to Stephan and he agreed we should leave this for now - perhaps when fingerprint API gets an update we can fix this but for now simply warning the class is not thread-safe will resolve the issue.

     
  • John May

    John May - 2013-09-23

    On branch with other patch - patches/p592+603

    Tiny 1 commit and I was too lazy to push two branches :-).

     
  • John May

    John May - 2013-09-24
    • status: open --> closed
     

Log in to post a comment.