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

Iterate over symbols to get the number to avoid array creation #101

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

krzysiek1507
Copy link

A tiny optimisation that reduces the memory usage by eliminating an array that it not used later anyway.

For parsing spec/fixtures/context/basic.y it reduces one object:

Before:

Calculating -------------------------------------
              master   133.668k memsize (     2.481k retained)
                         1.976k objects (     1.000  retained)
                        50.000  strings (     1.000  retained)

After:

Calculating -------------------------------------
              master   133.348k memsize (     2.481k retained)
                         1.975k objects (     1.000  retained)
                        50.000  strings (     1.000  retained)

@krzysiek1507
Copy link
Author

Hey @yui-knk is this PR worth merging?

@krzysiek1507 krzysiek1507 force-pushed the krzysiek1507/optimize-used-numbers branch from 56a16e2 to d57654c Compare October 7, 2023 12:09
@yui-knk
Copy link
Collaborator

yui-knk commented Oct 8, 2023

@krzysiek1507 Thanks for your PR. However I think this is not bottleneck of current Lrama. If it's not bottleneck, I want to keep current codes because it's intuitive for me.

By the way, this is how to take profiling for real world use case, generating parser for ruby/ruby.

1. Create parse.tmp.y in ruby/ruby

$ ruby tool/id2token.rb parse.y > parse.tmp.y
$ cp parse.tmp.y dir/lrama/tmp

2. Enable Profiler

diff --git a/exe/lrama b/exe/lrama
index ba5fb06..2497178 100755
--- a/exe/lrama
+++ b/exe/lrama
@@ -3,4 +3,6 @@
 $LOAD_PATH << File.join(__dir__, "../lib")
 require "lrama"

-Lrama::Command.new.run(ARGV.dup)
+Lrama::Report::Profile.report_profile do
+  Lrama::Command.new.run(ARGV.dup)
+end

3. Run Lrama

$ bundle exec exe/lrama -d -o parse.tmp.c -h tmp/parse.tmp.y

4. Generate Flamegraph

$ stackprof --d3-flamegraph tmp/stackprof-cpu-myapp.dump > tmp/flamegraph.html

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