You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by "dylanhz (via GitHub)" <gi...@apache.org> on 2026/02/14 08:07:48 UTC
[PR] [FLINK-39094][table-planner] Avoid creating duplicate function instances in code generation [flink]
dylanhz opened a new pull request, #27613:
URL: https://github.com/apache/flink/pull/27613
## What is the purpose of the change
Remove duplicate function instances in code generation.
## Brief change log
Add a `reusableFunctionTerms` set in `CodeGeneratorContext` to cache function field terms, preventing redundant deep copies when the same function is added multiple times, similar to reusable converters and type serializers.
## Verifying this change
Existing tests are enough.
## Does this pull request potentially affect one of the following parts:
- Dependencies (does it add or upgrade a dependency): (no)
- The public API, i.e., is any changed class annotated with `@Public(Evolving)`: (no)
- The serializers: (no)
- The runtime per-record code paths (performance sensitive): (no)
- Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: (no)
- The S3 file system connector: (no)
## Documentation
- Does this pull request introduce a new feature? (no)
- If yes, how is the feature documented? (not documented)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-39094][table-planner] Avoid creating duplicate function instances in code generation [flink]
Posted by "featzhang (via GitHub)" <gi...@apache.org>.
featzhang commented on PR #27613:
URL: https://github.com/apache/flink/pull/27613#issuecomment-3904911412
Good Job on function performance
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-39094][table-planner] Avoid creating duplicate function instances in code generation [flink]
Posted by "davidradl (via GitHub)" <gi...@apache.org>.
davidradl commented on code in PR #27613:
URL: https://github.com/apache/flink/pull/27613#discussion_r2807771872
##########
flink-table/flink-table-planner/src/main/scala/org/apache/flink/table/planner/codegen/CodeGeneratorContext.scala:
##########
@@ -837,6 +840,12 @@ class CodeGeneratorContext(
contextArgs: Seq[String] = null): String = {
val classQualifier = function.getClass.getName
val fieldTerm = CodeGenUtils.udfFieldName(function)
+ // check if function has been added before to avoid duplicate function instances
Review Comment:
I know the text of the PR says that existing tests are sufficient. It would be great to add a method level unit test to test out the logic around this specific change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-39094][table-planner] Avoid creating duplicate function instances in code generation [flink]
Posted by "dylanhz (via GitHub)" <gi...@apache.org>.
dylanhz commented on PR #27613:
URL: https://github.com/apache/flink/pull/27613#issuecomment-3957334063
> What about different arguments? What about deterministic/non-deterministic?
This fix targets redundant deep copies in `references`, not the function creation or invocation logic. Both before and after this fix, only one function instance is actually in effect at runtime — the change simply avoids unnecessary duplicate copies being added to the references array.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-39094][table-planner] Avoid creating duplicate function instances in code generation [flink]
Posted by "dylanhz (via GitHub)" <gi...@apache.org>.
dylanhz commented on code in PR #27613:
URL: https://github.com/apache/flink/pull/27613#discussion_r2851227928
##########
flink-table/flink-table-planner/src/main/scala/org/apache/flink/table/planner/codegen/CodeGeneratorContext.scala:
##########
@@ -837,6 +840,12 @@ class CodeGeneratorContext(
contextArgs: Seq[String] = null): String = {
val classQualifier = function.getClass.getName
val fieldTerm = CodeGenUtils.udfFieldName(function)
+ // check if function has been added before to avoid duplicate function instances
Review Comment:
I added unit tests in `CodeGeneratorContextTest` that directly asserts `references.size` after calling `addReusableFunction` with the same/different functions, covering stateless dedup, stateful dedup, and stateful non-dedup cases.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-39094][table-planner] Avoid creating duplicate function instances in code generation [flink]
Posted by "dylanhz (via GitHub)" <gi...@apache.org>.
dylanhz commented on code in PR #27613:
URL: https://github.com/apache/flink/pull/27613#discussion_r2851273416
##########
flink-table/flink-table-planner/src/main/scala/org/apache/flink/table/planner/codegen/CodeGeneratorContext.scala:
##########
@@ -129,6 +129,9 @@ class CodeGeneratorContext(
// string_constant -> reused_term
private val reusableStringConstants: mutable.Map[String, String] = mutable.Map[String, String]()
+ // set of function instance term that will be added only once
+ private val reusableFunctionTerms: mutable.HashSet[String] = mutable.HashSet[String]()
+
Review Comment:
> how can we be sure in this?
The dedup key (fieldTerm) is derived from `functionIdentifier()`, which is the same key used by `addReusableObjectInternal` to generate member/init statements — so the dedup is consistent with existing behavior.
> Can we add a test with custom udf and counter inside checking that it was invoked once?
I added unit tests in `CodeGeneratorContextTest` that directly asserts `references.size` after calling `addReusableFunction` with the same/different functions, covering stateless dedup, stateful dedup, and stateful non-dedup cases.
As for a counter-based test, `addReusableObjectInternal` creates instances via `InstantiationUtil.deserializeObject` that bypasses constructors, and `open()` was already deduplicated by LinkedHashSet before this fix, so neither counter can distinguish the before/after behavior. Let me know if you have better idea.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-39094][table-planner] Avoid creating duplicate function instances in code generation [flink]
Posted by "xuyangzhong (via GitHub)" <gi...@apache.org>.
xuyangzhong merged PR #27613:
URL: https://github.com/apache/flink/pull/27613
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-39094][table-planner] Avoid creating duplicate function instances in code generation [flink]
Posted by "flinkbot (via GitHub)" <gi...@apache.org>.
flinkbot commented on PR #27613:
URL: https://github.com/apache/flink/pull/27613#issuecomment-3901382795
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "b67d6e7589d24eb727218ea6cc5ef6837c170c4d",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "b67d6e7589d24eb727218ea6cc5ef6837c170c4d",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* b67d6e7589d24eb727218ea6cc5ef6837c170c4d UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run azure` re-run the last Azure build
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-39094][table-planner] Avoid creating duplicate function instances in code generation [flink]
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on PR #27613:
URL: https://github.com/apache/flink/pull/27613#issuecomment-3904228907
What about different arguments?
What about deterministic/non-deterministic?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-39094][table-planner] Avoid creating duplicate function instances in code generation [flink]
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on code in PR #27613:
URL: https://github.com/apache/flink/pull/27613#discussion_r2809047950
##########
flink-table/flink-table-planner/src/main/scala/org/apache/flink/table/planner/codegen/CodeGeneratorContext.scala:
##########
@@ -129,6 +129,9 @@ class CodeGeneratorContext(
// string_constant -> reused_term
private val reusableStringConstants: mutable.Map[String, String] = mutable.Map[String, String]()
+ // set of function instance term that will be added only once
+ private val reusableFunctionTerms: mutable.HashSet[String] = mutable.HashSet[String]()
+
Review Comment:
how can we be sure in this?
Can we add a test with custom udf and counter inside checking that it was invoked once?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-39094][table-planner] Avoid creating duplicate function instances in code generation [flink]
Posted by "davidradl (via GitHub)" <gi...@apache.org>.
davidradl commented on PR #27613:
URL: https://github.com/apache/flink/pull/27613#issuecomment-3921999608
@flinkbot run azure
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org