Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: Reduced runtime overhead by avoiding unnecessary reflections #583

Merged
merged 2 commits into from
Jan 24, 2025

Conversation

SahibYar
Copy link
Contributor

Key Optimizations

  1. Removed Unnecessary Reflection:

    • Using reflect.TypeOf was unnecessary since Go’s type-switch allows type assertion directly. This also eliminates the need to call reflect.TypeOf(value).String().
  2. Avoided Redundant Type Assertions:

    • In the original code, types were asserted after determining them with reflection (e.g., v, _ := value.(string)). The type-switch already ensures the type, making this extra step unnecessary.
  3. Improved Initialization:

    • Used make(map[string]string) for initializing result.
  4. Code Clarity:

    • Using direct type assertions in the type-switch improves readability and makes the function easier to understand.

Result

This optimized version reduces runtime overhead (by avoiding unnecessary reflections) and simplifies the logic while maintaining the same functionality. It is also more idiomatic Go code.

###Testing
I ran make all after doing these changes, and here is the output

go-fastly % make all
==> Downloading Go module
==> Downloading development dependencies
Warning: semgrep 1.104.0 is already installed and up-to-date.
To reinstall 1.104.0, run:
  brew reinstall semgrep
==> Tidying module
==> Running gofmt
==> Fixing imports
==> Testing go-fastly
?       github.com/fastly/go-fastly/v9/fastly/domains   [no test files]
ok      github.com/fastly/go-fastly/v9/fastly   (cached)
ok      github.com/fastly/go-fastly/v9/fastly/domains/v1        (cached)
?       github.com/fastly/go-fastly/v9/fastly/products  [no test files]
ok      github.com/fastly/go-fastly/v9/fastly/image_optimizer_default_settings  (cached)
ok      github.com/fastly/go-fastly/v9/fastly/products/bot_management   (cached)
ok      github.com/fastly/go-fastly/v9/fastly/products/brotli_compression       (cached)
ok      github.com/fastly/go-fastly/v9/fastly/products/ddos_protection  (cached)
ok      github.com/fastly/go-fastly/v9/fastly/products/domain_inspector (cached)
ok      github.com/fastly/go-fastly/v9/fastly/products/fanout   (cached)
ok      github.com/fastly/go-fastly/v9/fastly/products/image_optimizer  (cached)
ok      github.com/fastly/go-fastly/v9/fastly/products/log_explorer_insights    (cached)
ok      github.com/fastly/go-fastly/v9/fastly/products/ngwaf    (cached)
ok      github.com/fastly/go-fastly/v9/fastly/products/origin_inspector (cached)
ok      github.com/fastly/go-fastly/v9/fastly/products/websockets       (cached)
?       github.com/fastly/go-fastly/v9/internal/test_utils      [no test files]
ok      github.com/fastly/go-fastly/v9/internal/productcore     (cached)
==> Running go vet
==> Running staticcheck
staticcheck 2023.1.3 (v0.4.3)
if command -v semgrep &> /dev/null; then semgrep ci --config auto --exclude-rule generic.secrets.security.detected-private-key.detected-private-key ; fi
                  
                  
┌────────────────┐
│ Debugging Info │
└────────────────┘
                  
  SCAN ENVIRONMENT
  versions    - semgrep 1.104.0 on python 3.13.1                       
  environment - running in environment git, triggering event is unknown
                                                                                                                        
  Scanning 1263 files (only git-tracked) with 1057 Code rules:
            
  CODE RULES
                                                                                                                        
  Language      Rules   Files          Origin      Rules                                                                
 ─────────────────────────────        ───────────────────                                                               
  <multilang>      47    1144          Community    1057                                                                
  yaml             31     980                                                                                           
  go               83     145                                                                                           
  bash              4       5                                                                                           
                                                                                                                        
                    
  SUPPLY CHAIN RULES
                  
  No rules to run.
                  
          
  PROGRESS
   
  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 100% 0:00:00                                                                                                                        
                
                
┌──────────────┐
│ Scan Summary │
└──────────────┘
Some files were skipped or only partially analyzed.
  Scan was limited to files tracked by git.
  Partially scanned: 1 files only partially analyzed due to parsing or internal Semgrep errors
  Scan skipped: 1 files larger than 1.0 MB, 118 files matching .semgrepignore patterns
  For a full list of skipped files, run semgrep with the --verbose flag.

(need more rules? `semgrep login` for additional free Semgrep Registry rules)

CI scan completed successfully.
  Found 0 findings (0 blocking) from 1057 rules.
  No blocking findings so exiting with code 0

@kpfleming kpfleming merged commit e9d4a62 into fastly:main Jan 24, 2025
3 checks passed
@kpfleming kpfleming changed the title Reduced runtime overhead by avoiding unnecessary reflections refactor: Reduced runtime overhead by avoiding unnecessary reflections Jan 27, 2025
@SahibYar SahibYar deleted the refactor/removeReflection branch January 28, 2025 17:55
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.

2 participants