Skip to content

refactor: shrink SBuiltinFuncDefinition to reduce binary size by 17MB…#35322

Open
guanshengliang wants to merge 2 commits into
mainfrom
chore/shrink-size
Open

refactor: shrink SBuiltinFuncDefinition to reduce binary size by 17MB…#35322
guanshengliang wants to merge 2 commits into
mainfrom
chore/shrink-size

Conversation

@guanshengliang
Copy link
Copy Markdown
Contributor

… per executable

Description

Issue(s)

  • Close/close/Fix/fix/Resolve/resolve: Issue Link

Checklist

Please check the items in the checklist if applicable.

  • Is the user manual updated?
  • Are the test cases passed and automated?
  • Is there no significant decrease in test coverage?

Copilot AI review requested due to automatic review settings May 12, 2026 09:10
@guanshengliang guanshengliang requested a review from dapan1121 as a code owner May 12, 2026 09:10
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR reduces the memory footprint of builtin function metadata by shrinking the parameter-pattern arrays in SFunctionParaInfo, aiming to reduce per-executable binary size.

Changes:

  • Replace the previous MAX_FUNC_PARA_NUM sizing with separate limits for pattern count and pattern-entry count.
  • Reduce MAX_FUNC_PARA_FIXED_VALUE_NUM from 16 to 8.
  • Update SFunctionParaInfo::inputParaInfo dimensions to match the new sizing macros.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +33 to +35
#define MAX_FUNC_PARA_PATTERN_NUM 2
#define MAX_FUNC_INPUT_PARA_NUM 8
#define MAX_FUNC_PARA_FIXED_VALUE_NUM 8
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors function parameter limits in builtins.h by replacing the generic MAX_FUNC_PARA_NUM with more specific constants, MAX_FUNC_PARA_PATTERN_NUM and MAX_FUNC_INPUT_PARA_NUM, and reducing the value of MAX_FUNC_PARA_FIXED_VALUE_NUM. The SFunctionParaInfo struct was updated accordingly to use these new dimensions. Review feedback suggests documenting the rationale behind these specific constant values to improve code maintainability.

Comment on lines +33 to +35
#define MAX_FUNC_PARA_PATTERN_NUM 2
#define MAX_FUNC_INPUT_PARA_NUM 8
#define MAX_FUNC_PARA_FIXED_VALUE_NUM 8
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

These new macros act as magic numbers. To improve maintainability, please add comments explaining why these specific values were chosen. This context will be valuable for anyone modifying this code in the future.

Suggested change
#define MAX_FUNC_PARA_PATTERN_NUM 2
#define MAX_FUNC_INPUT_PARA_NUM 8
#define MAX_FUNC_PARA_FIXED_VALUE_NUM 8
#define MAX_FUNC_PARA_PATTERN_NUM 2 // Max parameter patterns per function. Currently 1 is used, 2 allows for future extension.
#define MAX_FUNC_INPUT_PARA_NUM 8 // Max parameter descriptors per pattern. `_irate_partial` uses 4.
#define MAX_FUNC_PARA_FIXED_VALUE_NUM 8 // Max fixed values for a parameter. `week` function uses 8.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants