From 331305abf5e6f55327b3c38531e77abc8edf478b Mon Sep 17 00:00:00 2001 From: Christopher Jones Date: Tue, 29 Sep 2015 19:51:10 +0200 Subject: [PATCH] Removed inappropriate use of SerialTaskQueue from ClusterShapeLazyGetter A SerialTaskQueue was added to ClusterShapeLazyGetter in an attempt to make it thread safe. Unfortunately, the implementation was not safe since the internals of SiPixelClusterShapeCache could be changed while another thread was reading the data outside of the queue. In addition, performance measurements done using VTune showed the fequent calls to SerialTaskQueue were causing a heavy CPU load when run with 8 threads. The thread safety of SiPixelClusterShapeCache will have to be handled in a different way in the future once we start using more than one thread per Event. --- .../plugins/SiPixelClusterShapeCacheProducer.cc | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/RecoPixelVertexing/PixelLowPtUtilities/plugins/SiPixelClusterShapeCacheProducer.cc b/RecoPixelVertexing/PixelLowPtUtilities/plugins/SiPixelClusterShapeCacheProducer.cc index 8e426753bf7de..935f81c96c8b8 100644 --- a/RecoPixelVertexing/PixelLowPtUtilities/plugins/SiPixelClusterShapeCacheProducer.cc +++ b/RecoPixelVertexing/PixelLowPtUtilities/plugins/SiPixelClusterShapeCacheProducer.cc @@ -3,7 +3,6 @@ #include "FWCore/Framework/interface/Event.h" #include "FWCore/Framework/interface/EventSetup.h" #include "FWCore/Utilities/interface/EDGetToken.h" -#include "FWCore/Concurrency/interface/SerialTaskQueue.h" #include "FWCore/ParameterSet/interface/ParameterSet.h" #include "FWCore/Utilities/interface/InputTag.h" @@ -31,18 +30,15 @@ namespace { ~ClusterShapeLazyGetter() {} void fill(const SiPixelClusterShapeCache::ClusterRef& cluster, const PixelGeomDetUnit *pixDet, const SiPixelClusterShapeCache& constCache) const override { - taskQueue_.pushAndWait([this, &cluster, pixDet, &constCache]{ - if(constCache.isFilled(cluster)) - return; - SiPixelClusterShapeCache& cache = const_cast(constCache); - this->data_.size.clear(); - this->clusterShape_.determineShape(*pixDet, *cluster, this->data_); - cache.insert(cluster, this->data_); - }); + if(constCache.isFilled(cluster)) + return; + SiPixelClusterShapeCache& cache = const_cast(constCache); + this->data_.size.clear(); + this->clusterShape_.determineShape(*pixDet, *cluster, this->data_); + cache.insert(cluster, this->data_); } private: - mutable edm::SerialTaskQueue taskQueue_; // not sure if this is the best synchronization mechanism mutable ClusterData data_; // reused mutable ClusterShape clusterShape_; };