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

    https://github.com/apache/spark/pull/19811#discussion_r154030837
  
    --- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala
 ---
    @@ -177,11 +190,67 @@ class CodegenContext {
        *                 the list of default imports available.
        *                 Also, generic type arguments are accepted but ignored.
        * @param variableName Name of the field.
    -   * @param initCode The statement(s) to put into the init() method to 
initialize this field.
    +   * @param codeFunctions Function includes statement(s) to put into the 
init() method to
    +   *                 initialize this field. An argument is the name of the 
mutable state variable
        *                 If left blank, the field will be default-initialized.
    +   * @param inline whether the declaration and initialization code may be 
inlined rather than
    +   *               compacted. If true, the name is not changed
    +   * @return the name of the mutable state variable, which is either the 
original name if the
    +   *         variable is inlined to the outer class, or an array access if 
the variable is to be
    +   *         stored in an array of variables of the same type and 
initialization.
    +   *         primitive type variables will be inlined into outer class 
when the total number of
    +   *         mutable variables is less than 
`CodeGenerator.OUTER_CLASS_VARIABLES_THRESHOLD`
    +   *         the max size of an array for compaction is given by
    +   *         `CodeGenerator.MUTABLESTATEARRAY_SIZE_LIMIT`.
        */
    -  def addMutableState(javaType: String, variableName: String, initCode: 
String = ""): Unit = {
    -    mutableStates += ((javaType, variableName, initCode))
    +  def addMutableState(
    +      javaType: String,
    +      variableName: String,
    +      codeFunctions: String => String = _ => "",
    +      inline: Boolean = false): String = {
    +    val varName = if (!inline) freshName(variableName) else variableName
    +    val initCode = codeFunctions(varName)
    +
    +    if (inline ||
    +        // want to put a primitive type variable at outerClass for 
performance
    +        isPrimitiveType(javaType) &&
    +          (mutableStates.length < 
CodeGenerator.OUTER_CLASS_VARIABLES_THRESHOLD) ||
    +        // identify multi-dimensional array or no simply-assigned object
    +        !isPrimitiveType(javaType) &&
    +          (javaType.contains("[][]") ||
    +           !initCode.matches("(^[\\w_]+\\d+\\s*=\\s*null;|"
    +              + "^[\\w_]+\\d+\\s*=\\s*new\\s*[\\w\\.]+\\(\\);$|"
    +              + "^$)"))) {
    +      // primitive type or non-simply-assigned state is declared inline to 
the outer class
    +      mutableStates += ((javaType, varName, initCode))
    +      varName
    +    } else {
    +      // Create an initialization code agnostic to the actual variable 
name which we can key by
    +      val initCodeKey = initCode.replaceAll(varName, "*VALUE*")
    --- End diff --
    
    Sorry for my 
    There are two cases.
    1. ```
    v => {
      nameInGlobal = ctx.fresh("tmp")
      $v = $name + 1;
    }
    ```
    2. 
    v => {
      val name = ctx.fresh("tmp")
      hashInGlobal += name -> v
      $v = $name + 1;
    }
    ```
    3. other cases
    
    You are right. I have not seen it now, but we cannot guarantee it would not 
happen. 
    
    My question is that what problem you saw by reusing the result of 
`codeFunctions(varName)`? Would it be possible to share it among us?


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

Reply via email to