Skip to content

refactor!: move IDL* types into the candid crate#622

Merged
ilbertt merged 11 commits intomasterfrom
luca/candid-idl-types
Jun 25, 2025
Merged

refactor!: move IDL* types into the candid crate#622
ilbertt merged 11 commits intomasterfrom
luca/candid-idl-types

Conversation

@ilbertt
Copy link
Copy Markdown
Contributor

@ilbertt ilbertt commented Jun 17, 2025

Overview
Move the parser's IDL* types to the candid crate, in order to share it between both the candid and the candid_parser crates.

The main reason is that we want to use the IDL* types in the bindings generators, instead of bringing around the TypeEnv, which holds inner Types that are not meant for codegen.

Considerations
Introduces breaking changes.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 17, 2025

Name Max Mem (Kb) Encode Decode
blob 4_224 4_206_540 2_121_052
btreemap 75_456 4_261_360_651 15_229_108_210
nns 128 1_964_633 5_458_175 ($\textcolor{green}{-0.28\%}$)
nns_list_proposal 1_088 6_829_375 ($\textcolor{green}{-0.27\%}$) 67_489_699 ($\textcolor{red}{0.00\%}$)
option_list 128 7_481_613 ($\textcolor{red}{0.01\%}$) 25_909_653 ($\textcolor{red}{0.98\%}$)
text 6_336 4_203_487 7_877_208
variant_list 128 7_537_050 ($\textcolor{red}{0.00\%}$) 24_319_827 ($\textcolor{red}{0.35\%}$)
vec_int16 16_704 123_692_964 1_069_570_572
  • Parser cost: 15_006_070 ($\textcolor{green}{-0.00\%}$)
  • Extra args: 3_161_072 ($\textcolor{green}{-0.00\%}$)
Click to see raw report
---------------------------------------------------

Benchmark: blob
  total:
    instructions: 6.33 M (no change)
    heap_increase: 66 pages (no change)
    stable_memory_increase: 0 pages (no change)

  1. Encoding (scope):
    calls: 1 (no change)
    instructions: 4.21 M (no change)
    heap_increase: 66 pages (no change)
    stable_memory_increase: 0 pages (no change)

  2. Decoding (scope):
    calls: 1 (no change)
    instructions: 2.12 M (no change)
    heap_increase: 0 pages (no change)
    stable_memory_increase: 0 pages (no change)

---------------------------------------------------

Benchmark: text
  total:
    instructions: 12.08 M (no change)
    heap_increase: 99 pages (no change)
    stable_memory_increase: 0 pages (no change)

  1. Encoding (scope):
    calls: 1 (no change)
    instructions: 4.20 M (no change)
    heap_increase: 66 pages (no change)
    stable_memory_increase: 0 pages (no change)

  2. Decoding (scope):
    calls: 1 (no change)
    instructions: 7.88 M (no change)
    heap_increase: 33 pages (no change)
    stable_memory_increase: 0 pages (no change)

---------------------------------------------------

Benchmark: vec_int16
  total:
    instructions: 1.19 B (no change)
    heap_increase: 261 pages (no change)
    stable_memory_increase: 0 pages (no change)

  1. Encoding (scope):
    calls: 1 (no change)
    instructions: 123.69 M (no change)
    heap_increase: 261 pages (no change)
    stable_memory_increase: 0 pages (no change)

  2. Decoding (scope):
    calls: 1 (no change)
    instructions: 1.07 B (no change)
    heap_increase: 0 pages (no change)
    stable_memory_increase: 0 pages (no change)

---------------------------------------------------

Benchmark: btreemap
  total:
    instructions: 19.49 B (no change)
    heap_increase: 1179 pages (no change)
    stable_memory_increase: 0 pages (no change)

  1. Encoding (scope):
    calls: 1 (no change)
    instructions: 4.26 B (no change)
    heap_increase: 159 pages (no change)
    stable_memory_increase: 0 pages (no change)

  2. Decoding (scope):
    calls: 1 (no change)
    instructions: 15.23 B (no change)
    heap_increase: 1020 pages (no change)
    stable_memory_increase: 0 pages (no change)

---------------------------------------------------

Benchmark: option_list
  total:
    instructions: 33.39 M (0.76%) (change within noise threshold)
    heap_increase: 2 pages (no change)
    stable_memory_increase: 0 pages (no change)

  1. Encoding (scope):
    calls: 1 (no change)
    instructions: 7.48 M (0.01%) (change within noise threshold)
    heap_increase: 0 pages (no change)
    stable_memory_increase: 0 pages (no change)

  2. Decoding (scope):
    calls: 1 (no change)
    instructions: 25.91 M (0.98%) (change within noise threshold)
    heap_increase: 2 pages (no change)
    stable_memory_increase: 0 pages (no change)

---------------------------------------------------

Benchmark: variant_list
  total:
    instructions: 31.86 M (0.26%) (change within noise threshold)
    heap_increase: 2 pages (no change)
    stable_memory_increase: 0 pages (no change)

  1. Encoding (scope):
    calls: 1 (no change)
    instructions: 7.54 M (0.00%) (change within noise threshold)
    heap_increase: 0 pages (no change)
    stable_memory_increase: 0 pages (no change)

  2. Decoding (scope):
    calls: 1 (no change)
    instructions: 24.32 M (0.35%) (change within noise threshold)
    heap_increase: 2 pages (no change)
    stable_memory_increase: 0 pages (no change)

---------------------------------------------------

Benchmark: nns
  total:
    instructions: 23.27 M (-0.07%) (change within noise threshold)
    heap_increase: 2 pages (no change)
    stable_memory_increase: 0 pages (no change)

  0. Parsing (scope):
    calls: 1 (no change)
    instructions: 15.01 M (-0.00%) (change within noise threshold)
    heap_increase: 2 pages (no change)
    stable_memory_increase: 0 pages (no change)

  1. Encoding (scope):
    calls: 1 (no change)
    instructions: 1.96 M (no change)
    heap_increase: 0 pages (no change)
    stable_memory_increase: 0 pages (no change)

  2. Decoding (scope):
    calls: 1 (no change)
    instructions: 5.46 M (-0.28%) (change within noise threshold)
    heap_increase: 0 pages (no change)
    stable_memory_increase: 0 pages (no change)

---------------------------------------------------

Benchmark: nns_list_proposal
  total:
    instructions: 74.32 M (-0.03%) (change within noise threshold)
    heap_increase: 17 pages (no change)
    stable_memory_increase: 0 pages (no change)

  1. Encoding (scope):
    calls: 1 (no change)
    instructions: 6.83 M (-0.27%) (change within noise threshold)
    heap_increase: 3 pages (no change)
    stable_memory_increase: 0 pages (no change)

  2. Decoding (scope):
    calls: 1 (no change)
    instructions: 67.49 M (0.00%) (change within noise threshold)
    heap_increase: 14 pages (no change)
    stable_memory_increase: 0 pages (no change)

---------------------------------------------------

Benchmark: extra_args
  total:
    instructions: 3.16 M (-0.00%) (change within noise threshold)
    heap_increase: 0 pages (no change)
    stable_memory_increase: 0 pages (no change)

---------------------------------------------------

Summary:
  instructions:
    status:   No significant changes 👍
    counts:   [total 9 | regressed 0 | improved 0 | new 0 | unchanged 9]
    change:   [max +252.87K | p75 0 | median 0 | p25 -2 | min -18.71K]
    change %: [max +0.76% | p75 0.00% | median 0.00% | p25 -0.00% | min -0.07%]

  heap_increase:
    status:   No significant changes 👍
    counts:   [total 9 | regressed 0 | improved 0 | new 0 | unchanged 9]
    change:   [max 0 | p75 0 | median 0 | p25 0 | min 0]
    change %: [max 0.00% | p75 0.00% | median 0.00% | p25 0.00% | min 0.00%]

  stable_memory_increase:
    status:   No significant changes 👍
    counts:   [total 9 | regressed 0 | improved 0 | new 0 | unchanged 9]
    change:   [max 0 | p75 0 | median 0 | p25 0 | min 0]
    change %: [max 0.00% | p75 0.00% | median 0.00% | p25 0.00% | min 0.00%]

---------------------------------------------------
Successfully persisted results to canbench_results.yml

@ilbertt ilbertt marked this pull request as ready for review June 17, 2025 14:39
@ilbertt ilbertt requested a review from a team as a code owner June 17, 2025 14:39
Copy link
Copy Markdown
Contributor

@christoph-dfinity christoph-dfinity left a comment

Choose a reason for hiding this comment

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

Looks good to me. One bikeshed comment would be that src/types/syntax.rs rather than src/types/parser.rs might be a nicer name, as the idea is that it describes the syntax of candid files. Not just the parser is a consumer of this format, but also everyone that wants to generate a .did file.

@ilbertt
Copy link
Copy Markdown
Contributor Author

ilbertt commented Jun 18, 2025

Looks good to me. One bikeshed comment would be that src/types/syntax.rs rather than src/types/parser.rs might be a nicer name, as the idea is that it describes the syntax of candid files. Not just the parser is a consumer of this format, but also everyone that wants to generate a .did file.

@christoph-dfinity good point. I've renamed both the module and the feature flag accordingly.

@ilbertt
Copy link
Copy Markdown
Contributor Author

ilbertt commented Jun 18, 2025

Since this PR introduces breaking changes, I'll wait before merging it to check if the changes make sense in the branch I'm working on: https://github.com/dfinity/candid/compare/luca/idl-types-in-parser

@ilbertt ilbertt changed the title refactor!: move IDL* types into the candid crate, under the parser feature flag refactor!: move IDL* types into the candid crate Jun 20, 2025
@ilbertt
Copy link
Copy Markdown
Contributor Author

ilbertt commented Jun 20, 2025

@christoph-dfinity I realized I need the syntax module inside candid for the export_service macro, so I've removed the feature.

@lwshang
Copy link
Copy Markdown
Contributor

lwshang commented Jun 24, 2025

Luca and I discussed the upcoming changes offline.

We've decided to proceed with the current PR structure for now, given the dependent work (e.g., #625). However, we'll plan to move the types into a separate, privately-dependent crate (could be named candid_syntax) before the final release.

Since development in this repo has been slow, we can continue using the master branch as our active development branch and aren't expecting any patch releases soon.

@ilbertt ilbertt merged commit b1d4a57 into master Jun 25, 2025
11 checks passed
@ilbertt ilbertt deleted the luca/candid-idl-types branch June 25, 2025 18:38
ilbertt added a commit that referenced this pull request Jul 21, 2025
ilbertt added a commit that referenced this pull request Jul 21, 2025
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.

3 participants