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

Serialized instead of parallel path finding #348

Closed
wants to merge 1 commit into from

Conversation

hendrikvanantwerpen
Copy link
Collaborator

This PR implements an idea I raised in #346: instead of processing all initial paths in parallel, process them in serial.

Note that I've made that change for find_minimal_partial_path_set_in_file and find_all_complete_partial_paths, not the stitcher itself. Especialyl the former is often called with initial paths for all clickable nodes in the graph, and the effect on max memory usage should be noticable, I hope.

@hendrikvanantwerpen hendrikvanantwerpen requested a review from a team as a code owner November 13, 2023 18:20
@hendrikvanantwerpen hendrikvanantwerpen self-assigned this Nov 13, 2023
Copy link

Performance Summary

Comparing base cd58abb with head 04aebfc on typescript_benchmark benchmark. For details see workflow artifacts. Note that performance is tested on the last commits with changes in stack-graphs, not on every commit.

Before
--------------------------------------------------------------------------------
Command:            base/target/release/tree-sitter-stack-graphs-typescript index -D base.sqlite --max-file-time=30 --hide-error-details -- base/data/typescript_benchmark
Massif arguments:   --massif-out-file=perf.out
ms_print arguments: --x=72 --y=12 base-perf-results/perf.out
--------------------------------------------------------------------------------


    GB
3.276^                                                                     :# 
     |                                                                     :# 
     |                                                                     :# 
     |                                                                     :#:
     |                                                                     :#:
     |                                                                     :#:
     |                                             :::@                : :@:#:
     |                                             :  @                :::@:#:
     |                                          @@@:  @               ::::@:#:
     |                                       :::@  :  @               ::::@:#:
     |                                 @:   ::  @  :  @        ::@:  @::::@:#:
     |              :  :@@     :    ::@@:: @::  @  :  @       :::@:::@::::@:#:
   0 +----------------------------------------------------------------------->Gi
     0                                                                   74.84
After
--------------------------------------------------------------------------------
Command:            head/target/release/tree-sitter-stack-graphs-typescript index -D head.sqlite --max-file-time=30 --hide-error-details -- head/data/typescript_benchmark
Massif arguments:   --massif-out-file=perf.out
ms_print arguments: --x=72 --y=12 head-perf-results/perf.out
--------------------------------------------------------------------------------


    MB
715.3^                                                           ##           
     |                                                           #            
     |                                                           #            
     |                                 @@:                     ::#            
     |                                 @ :           ::        ::#            
     |                                 @ :           ::        ::#            
     |                                 @ :::    :::::::        ::#            
     |              :     :            @ :::    ::: :::       @::# ::     ::::
     |              :::   :            @ :::  ::::: :::       @::# :    ::::::
     |              ::  ::: :         :@ :::@@::::: :::       @::# : :::::::::
     |              :: :: : ::  :::   :@ :::@ ::::: ::::  ::  @::# : :::::::::
     |             @:: :: : ::@@: ::  :@ :::@ ::::: ::::  : ::@::# : :::::::::
   0 +----------------------------------------------------------------------->Gi
     0                                                                   70.72

@hendrikvanantwerpen
Copy link
Collaborator Author

Hm, there seems some variation in those profiling runs, because the before graph is much higher than the after graph in #346 , even though they are supposed to be based on the same commit 😕.

Comparing the after graphs of this PR and #346 suggests maybe a slight decrease in memory usage? I'll do some more local experiments to see if it shows better for e.g. larger files.

@hendrikvanantwerpen hendrikvanantwerpen marked this pull request as draft November 13, 2023 21:37
@hendrikvanantwerpen
Copy link
Collaborator Author

Closing this old experiment.

@hendrikvanantwerpen hendrikvanantwerpen deleted the serialize-path-finding branch March 12, 2024 14:58
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.

1 participant