Skip to content

[SPARK-58637][ML] Optimize HashingTF transform closure - #57845

Closed
zhengruifeng wants to merge 1 commit into
apache:masterfrom
zhengruifeng:SPARK-58637
Closed

[SPARK-58637][ML] Optimize HashingTF transform closure#57845
zhengruifeng wants to merge 1 commit into
apache:masterfrom
zhengruifeng:SPARK-58637

Conversation

@zhengruifeng

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

This PR optimizes HashingTF.transform by selecting the binary/counting UDF and hash function
from ($(binary), hashFuncVersion) before executing the UDF. The UDF calculates feature indexes
from the captured hash function and local numFeatures value instead of calling the instance
indexOf method.

Binary mode uses OpenHashMap.update, while counting mode uses changeValue. Both store integer
term counts and convert them to Double only when constructing the output sparse vector.

The compatibility test for HashingTF models saved before Spark 3.0 now also verifies transform,
including the saved binary mode and legacy hash function.

Why are the changes needed?

Calling indexOf from the UDF captures the HashingTF transformer and performs a parameter lookup
and hash-version match for every term. Selecting the behavior before constructing the UDF reduces
the closure to the values it needs, removes repeated branching and parameter lookups, and reduces
the term-count map value size from Double to Int.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

build/sbt -java-home /usr/lib/jvm/java-17-openjdk-amd64 'mllib/testOnly org.apache.spark.ml.feature.HashingTFSuite'

All 6 tests passed. The compiled UDF helper signatures were also inspected to confirm that they do
not retain a HashingTF receiver.

Was this patch authored or co-authored using generative AI tooling?

Generated-by: Codex (GPT-5)

@uros-b

uros-b commented Aug 7, 2026

Copy link
Copy Markdown
Member

Thank you @zhengruifeng!

@dongjoon-hyun

Copy link
Copy Markdown
Member

Nice optimization — pre-selecting the hash function and UDF avoids the per-term hashFuncVersion match and $(numFeatures) param lookup, and OpenHashMap[Int, Int] benefits from value specialization. I verified the semantics are unchanged: binary mode's update(index, 1) matches the old changeValue result, Vectors.sparse(n, Seq) sorts by index in both paths, and the eta-expanded object methods keep the closure free of the HashingTF receiver.

A few suggestions:

  1. The binary flag and hash version are independent axes, so the 2x2 match could be simplified to a hash-function match plus a binary branch, mirroring the existing indexOf structure:

    val hashFunc: Any => Int = hashFuncVersion match {
      case HashingTF.SPARK_2_MURMUR3_HASH => OldHashingTF.murmur3Hash
      case HashingTF.SPARK_3_MURMUR3_HASH => FeatureHasher.murmur3Hash
      case _ => throw new IllegalArgumentException("Illegal hash function version setting.")
    }
    val hashUDF = if ($(binary)) binaryHashUDF(hashFunc) else countHashUDF(hashFunc)
  2. Test coverage: with the new test, three of the four (binary, hashFuncVersion) combinations are exercised through transform; only (binary = true, SPARK_2_MURMUR3_HASH) remains uncovered. Adding a setBinary(true) comparison in the new test would close that path. Note the 2.4.4 model's binary is the default false, so mLlibHashingTF.setBinary(loadedHashingTF.getBinary) is effectively a no-op today.

  3. Nit: the sparse-vector construction is duplicated between the two UDF helpers, and new OpenHashMap[Int, Int] drops the () used elsewhere in this file.

zhengruifeng added a commit that referenced this pull request Aug 10, 2026
### What changes were proposed in this pull request?

This PR optimizes `HashingTF.transform` by selecting the binary/counting UDF and hash function
from `($(binary), hashFuncVersion)` before executing the UDF. The UDF calculates feature indexes
from the captured hash function and local `numFeatures` value instead of calling the instance
`indexOf` method.

Binary mode uses `OpenHashMap.update`, while counting mode uses `changeValue`. Both store integer
term counts and convert them to `Double` only when constructing the output sparse vector.

The compatibility test for HashingTF models saved before Spark 3.0 now also verifies `transform`,
including the saved binary mode and legacy hash function.

### Why are the changes needed?

Calling `indexOf` from the UDF captures the `HashingTF` transformer and performs a parameter lookup
and hash-version match for every term. Selecting the behavior before constructing the UDF reduces
the closure to the values it needs, removes repeated branching and parameter lookups, and reduces
the term-count map value size from `Double` to `Int`.

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

`build/sbt -java-home /usr/lib/jvm/java-17-openjdk-amd64 'mllib/testOnly org.apache.spark.ml.feature.HashingTFSuite'`

All 6 tests passed. The compiled UDF helper signatures were also inspected to confirm that they do
not retain a `HashingTF` receiver.

### Was this patch authored or co-authored using generative AI tooling?

Generated-by: Codex (GPT-5)

Closes #57845 from zhengruifeng/SPARK-58637.

Authored-by: Ruifeng Zheng <ruifengz@apache.org>
Signed-off-by: Ruifeng Zheng <ruifengz@foxmail.com>
(cherry picked from commit 8210537)
Signed-off-by: Ruifeng Zheng <ruifengz@foxmail.com>
@zhengruifeng

Copy link
Copy Markdown
Contributor Author

Merge Summary:

Posted by merge_spark_pr.py

@zhengruifeng
zhengruifeng deleted the SPARK-58637 branch August 10, 2026 04:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants