trafodion-codereview mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From zellerh <...@git.apache.org>
Subject [GitHub] incubator-trafodion pull request #1228: [TRAFODION-2733] Provide an improved...
Date Wed, 13 Sep 2017 02:26:39 GMT
Github user zellerh commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/1228#discussion_r138511601
  
    --- Diff: core/sql/generator/GenProbeCache.cpp ---
    @@ -470,30 +463,38 @@ CostScalar ProbeCache::getEstimatedRunTimeMemoryUsage(NABoolean
perCPU)
       }
       const double outputBufferSize = resultSize * numBufferPoolEntries;  // in bytes
     
    -  // totalMemory is perCPU at this point of time.
    +  // totalMemory is perNode at this point of time.
       double totalMemory = cacheSize + outputBufferSize;
    -
    -  if ( perCPU == FALSE ) {
    -     const PhysicalProperty* const phyProp = getPhysicalProperty();
    -
    -     if (phyProp != NULL)
    -     {
    -       PartitioningFunction * partFunc = phyProp -> getPartitioningFunction() ;
    -       totalMemory *= partFunc->getCountOfPartitions();
    -     }
    +  Lng32 numOfStreams = 1;
    +  const PhysicalProperty* const phyProp = getPhysicalProperty();
    +  if (phyProp)
    +  {
    +     PartitioningFunction * partFunc = phyProp -> getPartitioningFunction() ;
    +     numOfStreams = partFunc->getCountOfPartitions();
    +     if (numOfStreams <= 0)
    +        numOfStreams = 1;
    +     double  memoryLimitPerInstance =
    +          ActiveSchemaDB()->getDefaults().getAsLong(EXE_MEMORY_FOR_PROBE_CACHE_IN_MB)
* 1024 * 1024;
    +     if (totalMemory > memoryLimitPerInstance)
    +        totalMemory = memoryLimitPerInstance;          
    +     totalMemory *= numOfStreams;
       }
    -
    -  double  memoryLimitPerCpu =
    -      ActiveSchemaDB()->getDefaults().getAsLong(EXE_MEMORY_FOR_PROBE_CACHE_IN_MB)
* 1024 * 1024;
    -  if (totalMemory > memoryLimitPerCpu)
    -     totalMemory = memoryLimitPerCpu;          
    +  if (numStreams != NULL)
    +     *numStreams = numOfStreams;
    +  if ( perNode == TRUE ) 
    --- End diff --
    
    Yes, I guess you didn't write this, it's old code. As far as I understand, TRUE is defined
as 1. If perNode happens to be -1, we will not execute the if statement, but we usually want
to treat any value other than 0 as true. That's why I would either use if (perNode) or if
(perNode != FALSE).


---

Mime
View raw message