perf(npm/csharp): a child chain is derived for every node with a parameter list

Open Beginner friendly
#1,879 1 comment 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 1 day

Nobody has claimed this yet.

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
72/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
csharp, rust

Research direction

Read src/metrics/npm/csharp.rs and the Java guard described in the issue, then inspect the_member_walks_derive_a_chain_per_body_not_per_node and member_walk_siblings::FIXTURES in src/observation_tests.rs. Check the C# node kind before deriving a child chain, then extend the guard to C# or the fixture rows; done means the C# per body count matches the expected behavior without regressing the existing guard.

Written by the indexing model from the issue text.

Description

low-priority performance

Summary

C#'s Npm::compute derives a child chain for every node that has a parameter_list child, before it knows the node is a type declaration. A class with 65 methods derives 67 chains where the other languages derive 2.

Measured

Counted with child_chains_derived over Ast::metrics with only Npa and Npm selected, on a class body of 65 units. Measured on the local branch refactor/1871-sibling-followups (not pushed) at e0cc1918, whose base is 68b70b08; the C# code is unchanged from main:

Language Unit Chains derived
Java int x; and void m() {} 2
Kotlin val x = 1 and fun m() {} 2
C++ int x; and void m() {} 2
C# public int x; and void M() {} 67

The fixture table is member_walk_siblings::FIXTURES in src/observation_tests.rs.

Cause

src/metrics/npm/csharp.rs asks the primary-constructor question (#1828) of every node it is handed:

if let Some(list) = node.first_child(|id| id == ParameterList)
    && func_opener::is_callable(&list, code, ancestors.of_children(node).get())

A method_declaration has a parameter_list child, so the chain is derived for each method, then the row's parent gate answers "not under a type declaration" and the chain is dropped.

Cost

One allocation per method, constructor, local function and delegate. A derivation has been one allocation since #1805, so this is a constant factor that the scaling gate cannot price. It is the shape the_member_walks_derive_a_chain_per_body_not_per_node guards for Java only (lesson 103).

Suggested fix

Test the node's kind (the type declarations a primary constructor can sit under) before deriving the chain, or read the parent gate's answer from node itself. Then extend the per-body guard to C#, or to every row of the fixture table above.

Related

#1828, #1805, #1696, #1871.

Dominant language
Rust
Stars
32
Forks
3
Avg merge
9h 55m
Merged PRs (30d)
102

Getting set up

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from dekobon/big-code-analysis

All issues in dekobon/big-code-analysis

Similar issues

More Rust issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.