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

Compiler: Add "Top 10 slowest modules" to --stats output #12942

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

z64
Copy link
Contributor

@z64 z64 commented Jan 11, 2023

May be useful for spotting modules in large apps that may benefit from being broken up in order to improve build times, etc.

Example output:

Codegen (bc+obj):
 - 1757/1771 .o files were reused

These modules were not reused:
 - _main (_main.bc)
 - Process (P-rocess.bc)
 - /home/lune/git/crystal/src/compiler/crystal/macros/methods.cr (47home47lune47git-047426b817c2f879b1b5b500a3d97bbe.bc)
 - Crystal::Signal (C-rystal5858S-ignal.bc)
 - Crystal::Compiler (C-rystal5858C-ompiler.bc)
 - Hash(String, Int32) (H-ash40S-tring4432I-nt3241.bc)
 - Log::AsyncDispatcher (L-og5858A-syncD-ispatcher.bc)
 - Crystal::Command (C-rystal5858C-ommand.bc)
 - Crystal::Init (C-rystal5858I-nit.bc)
 - Crystal::Playground::Server (C-rystal5858P-layground5858S-erver.bc)
 - Crystal::Playground::Session (C-rystal5858P-layground5858S-ession.bc)
 - Crystal::Playground::PathWebSocketHandler (C-rystal5858P-lay-38c77a69c1a2ffd144caae7845092122.bc)
 - HTTP::Server (H-T-T-P-5858S-erver.bc)
 - HTTP::WebSocketHandler+ (H-T-T-P-5858W-ebS-ocketH-andler43.bc)

Top 10 slowest modules:
 - 00:00:00.794907504 _main (_main.bc)
 - 00:00:00.201288353 Crystal::Command (C-rystal5858C-ommand.bc)
 - 00:00:00.135537059 /home/lune/git/crystal/src/compiler/crystal/macros/methods.cr (47home47lune47git-047426b817c2f879b1b5b500a3d97bbe.bc)
 - 00:00:00.135338039 Crystal::Generic (C-rystal5858G-eneric.bc)
 - 00:00:00.111936502 Crystal::Compiler (C-rystal5858C-ompiler.bc)
 - 00:00:00.104652722 Crystal::ASTNode+ (C-rystal5858A-S-T-N-ode43.bc)
 - 00:00:00.091841367 Crystal::CodeGenVisitor (C-rystal5858C-odeG-enV-isitor.bc)
 - 00:00:00.083189850 Crystal::Call (C-rystal5858C-all.bc)
 - 00:00:00.075213115 Crystal::Type+ (C-rystal5858T-ype43.bc)
 - 00:00:00.066328602 Crystal::NumberLiteral (C-rystal5858N-umberL-iteral.bc)

Comment on lines +477 to +480
slice.update(module_idx) do |unit|
unit.compilation_time = unit_info["time"].as_f.seconds
unit
end
Copy link
Contributor

Choose a reason for hiding this comment

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

unit is a reference, so I believe direct mutation works and there is no need to do it like a value (unless there is some complication with forking)

Copy link
Contributor Author

@z64 z64 Jan 31, 2023

Choose a reason for hiding this comment

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

Pages in forked processes are copy-on-write by default. And the pages allocated by bdwgc are not marked as MEM_SHARED (probably for good reason, I'd wager). That is why this pipe mechanism is used, as a safe vehicle to share memory.


Edit: Sorry, mistook the question; Yes - in this context we are in the parent thread, so mutating should be fine here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Are you still going to address this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@HertzDevil No, make any changes you see fit.

@@ -631,6 +652,7 @@ module Crystal
getter original_name
getter llvm_mod
getter? reused_previous_compilation = false
property compilation_time : Time::Span = Time::Span::ZERO
Copy link
Contributor

Choose a reason for hiding this comment

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

This should probably be a property! instead

@HertzDevil HertzDevil self-assigned this Jun 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants