refactor declaration.rs to change conventions by jesse-shopify · Pull Request #425 · Shopify/rubydex

jesse-shopify · 2026-01-08T21:42:40Z

remove macros
remove individual classes as they currently do not have data or behavior that differs from their shared components - when/if they do they can be re-added
use enum Declaration to separate types
implement helper methods to get at useful structs on enum so match statements don't proliferate. can use:
- as_simple
- as_simple_mut
- as_namespace
- as_namespace_mut

jesse-shopify · 2026-01-08T21:47:29Z

rust/rubydex/src/model/graph.rs

-                    // TODO: temporary hack to avoid crashing on `Struct.new`, `Class.new` and `Module.new`
-                }
-                _ => panic!("Tried to add member to a declaration that isn't a namespace"),
+            if let Some(namespace) = declaration.as_namespace_mut() {


cleaner, I think

jesse-shopify · 2026-01-09T00:01:33Z

rust/rubydex/src/resolution.rs

-    }
+    decl.as_namespace()
+        .expect("Tried to get members for a declaration that isn't a namespace")
+        .members()


Morriar · 2026-01-09T14:52:35Z

rust/rubydex/src/model/declaration.rs

+#[derive(Debug)]
+pub struct NamespaceDeclaration {
+    /// The base simple declaration fields
+    simple: SimpleDeclaration,


I tried this design earlier in the project and having this extra field for all definitions resulted in a lot more memory usage.

Could you compare main against your branch with --stats on a few corpus sizes?

You can produce test corpuses with https://github.com/Shopify/saturn/blob/main/utils/generate-corpus#L11.

The data is below. There was no change in memory or runtime performance.

In terms of API, I'm not a fan of having to go through simple depending on the field I want to access.

On the other hand, I do like the simplification around handling namespace related fields and avoiding the matches/panic. Could we get the same result with nested enums instead?

The thing about Rust is that there are many ways to get to the same place and the only different is ergonomics. Nested enums could work. Another option to improve ergonomics might be Using Deref / DerefMut traits. Another option would be to have a SimpleDeclaration that has an optional, boxed NamespaceDeclaration, though I suppose that would add an extra 8 bytes to SimpleDeclaration but then Declaration might not need to box which could be a performance and memory win.

I tried nested enums in #470, it's not horrible.

…st conventions

jesse-shopify · 2026-01-13T18:13:42Z

refactor declaration.rs to change conventions #425 👈 (View in Graphite)
main

This stack of pull requests is managed by Graphite. Learn more about stacking.

jesse-shopify · 2026-01-13T18:15:15Z

Memory usage using huge corpus:

Main

----------------------------------------
Indexed 100000 files
Found 13998368 names
Found 14007331 definitions
Found 100000 URIs
----------------------------------------
Maximum Resident Set Size: 13260619776 bytes (12646.31 MB)
Peak Memory Footprint:     10639665448 bytes (10146.77 MB)
Execution Time:            33.55 seconds

This Branch

----------------------------------------
Indexed 100000 files
Found 13998368 names
Found 14007331 definitions
Found 100000 URIs
----------------------------------------
Maximum Resident Set Size: 12421169152 bytes (11845.75 MB)
Peak Memory Footprint:     10664568888 bytes (10170.52 MB)
Execution Time:            33.99 seconds

No significant change in memory usage.

small refactors

jesse-shopify · 2026-01-13T18:25:20Z

rust/rubydex/Cargo.toml

 test_utils = ["dep:tempfile"]

 [dependencies]
+parking_lot = "0.12"


Added due to better lock API

jesse-shopify · 2026-01-13T18:33:27Z

main stats


Query statistics
  Total declarations:         13998368
  With documentation:         1643532 (11.7%)
  Without documentation:      12354836 (88.3%)
  Total documentation size:   1897179 bytes
  Multi-definition names:     8401 (0.1%)

Definition breakdown:
  Method               10000000
  InstanceVariable     2000446
  Constant             1000000
  Class                599649
  Module               300627
  SingletonClass        99724
  ClassVariable          5410
  GlobalVariable          920

Timing breakdown
  Listing             0.284s (  0.8%)
  Indexing            8.987s ( 24.6%)
  Resolution         22.851s ( 62.5%)
  Querying            4.451s ( 12.2%)
  Cleanup             0.000s (  0.0%)
  Total:             36.574s

Maximum RSS: 14091059200 bytes (13438.28 MB)

Indexed 100000 files
Found 13998368 names
Found 14007331 definitions
Found 100000 URIs

branch stats

Query statistics
  Total declarations:         13998368
  With documentation:         1643532 (11.7%)
  Without documentation:      12354836 (88.3%)
  Total documentation size:   1897179 bytes
  Multi-definition names:     8401 (0.1%)

Definition breakdown:
  Method               10000000
  InstanceVariable     2000446
  Constant             1000000
  Class                599649
  Module               300627
  SingletonClass        99724
  ClassVariable          5410
  GlobalVariable          920

Timing breakdown
  Listing             0.282s (  0.7%)
  Indexing            9.905s ( 25.8%)
  Resolution         23.626s ( 61.6%)
  Querying            4.546s ( 11.9%)
  Cleanup             0.000s (  0.0%)
  Total:             38.359s

Maximum RSS: 13771784192 bytes (13133.80 MB)

Indexed 100000 files
Found 13998368 names
Found 14007331 definitions
Found 100000 URIs

jesse-shopify commented Jan 8, 2026

View reviewed changes

jesse-shopify commented Jan 9, 2026

View reviewed changes

Morriar reviewed Jan 9, 2026

View reviewed changes

jesse-shopify self-assigned this Jan 12, 2026

jesse-shopify added 4 commits January 13, 2026 10:12

refactor declaration.rs to remove macro use and use more idiomatic Ru…

cf7efa6

…st conventions

minor touchups

49de5f9

fix

352dc74

fixes

4942fc6

jesse-shopify force-pushed the jj-refactor-declarations branch from decd579 to 4942fc6 Compare January 13, 2026 18:13

add poarking_lot to remove mutex/lock panics

7b8e4c0

small refactors

jesse-shopify commented Jan 13, 2026

View reviewed changes

jesse-shopify marked this pull request as ready for review January 14, 2026 19:29

jesse-shopify requested a review from a team as a code owner January 14, 2026 19:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor declaration.rs to change conventions#425

refactor declaration.rs to change conventions#425
jesse-shopify wants to merge 5 commits intomainfrom
jj-refactor-declarations

jesse-shopify commented Jan 8, 2026 •

edited

Loading

Uh oh!

jesse-shopify Jan 8, 2026

Uh oh!

jesse-shopify Jan 9, 2026

Uh oh!

Morriar Jan 9, 2026

Uh oh!

jesse-shopify Jan 13, 2026

Uh oh!

Morriar Jan 15, 2026 •

edited

Loading

Uh oh!

jesse-shopify Jan 15, 2026

Uh oh!

Morriar Jan 15, 2026

Uh oh!

jesse-shopify commented Jan 13, 2026

Uh oh!

jesse-shopify commented Jan 13, 2026

Uh oh!

jesse-shopify Jan 13, 2026

Uh oh!

jesse-shopify commented Jan 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

jesse-shopify commented Jan 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jesse-shopify Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

jesse-shopify Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

Morriar Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

jesse-shopify Jan 13, 2026

Choose a reason for hiding this comment

Uh oh!

Morriar Jan 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jesse-shopify Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

Morriar Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

jesse-shopify commented Jan 13, 2026

Uh oh!

jesse-shopify commented Jan 13, 2026

Main

This Branch

Uh oh!

jesse-shopify Jan 13, 2026

Choose a reason for hiding this comment

Uh oh!

jesse-shopify commented Jan 13, 2026

main stats

branch stats

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jesse-shopify commented Jan 8, 2026 •

edited

Loading

Morriar Jan 15, 2026 •

edited

Loading