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

Reduce number of function call on traverse #531

Merged
merged 1 commit into from
Feb 9, 2024

Conversation

ypconstante
Copy link
Contributor

Today traverse_with is implemented in a way to make the pattern matching easier and split into multiple calls. This is causing traverse_with to be called three times for each element, one to get the top of the stack, one to get the first element of the list, and another to do the actual processing of the element.

This PR updates traverse_with to use the stack directly, providing a small performance improvement and memory reduction

Finder benchmark

##### With input small #####
Name                             ips        average  deviation         median         99th %
id (pr)                       1.71 K        0.58 ms    ±17.13%        0.60 ms        0.84 ms
id (main)                     1.50 K        0.66 ms    ±21.56%        0.63 ms        1.32 ms
class (pr)                    1.29 K        0.77 ms    ±16.03%        0.82 ms        1.10 ms
class (main)                  1.26 K        0.79 ms    ±20.84%        0.75 ms        1.43 ms
tag name (type) (pr)          0.87 K        1.15 ms    ±14.08%        1.11 ms        1.73 ms
tag name (type) (main)        0.75 K        1.33 ms    ±21.38%        1.24 ms        2.50 ms
class multiple (pr)           0.70 K        1.42 ms    ±14.60%        1.37 ms        2.09 ms
class multiple (main)         0.65 K        1.53 ms    ±20.91%        1.43 ms        3.31 ms

Comparison: 
id (pr)                       1.71 K
id (main)                     1.50 K - 1.14x slower +0.0809 ms
class (pr)                    1.29 K - 1.33x slower +0.191 ms
class (main)                  1.26 K - 1.36x slower +0.21 ms
tag name (type) (pr)          0.87 K - 1.97x slower +0.57 ms
tag name (type) (main)        0.75 K - 2.28x slower +0.75 ms
class multiple (pr)           0.70 K - 2.43x slower +0.84 ms
class multiple (main)         0.65 K - 2.62x slower +0.95 ms

Memory usage statistics:

Name                           average  deviation         median         99th %
id (pr)                      619.99 KB     ±0.00%      619.99 KB      619.99 KB
id (main)                    667.04 KB     ±0.00%      667.04 KB      667.04 KB
class (pr)                   625.28 KB     ±0.00%      625.28 KB      625.33 KB
class (main)                 672.33 KB     ±0.00%      672.33 KB      672.33 KB
tag name (type) (pr)         802.34 KB     ±0.00%      802.34 KB      802.34 KB
tag name (type) (main)       849.46 KB     ±0.00%      849.46 KB      849.46 KB
class multiple (pr)          744.91 KB     ±0.00%      744.91 KB      744.95 KB
class multiple (main)        838.79 KB     ±0.00%      838.79 KB      838.79 KB

Comparison: 
id (pr)                      619.99 KB
id (main)                    667.04 KB - 1.08x memory usage +47.05 KB
class (pr)                   625.28 KB - 1.01x memory usage +5.29 KB
class (main)                 672.33 KB - 1.08x memory usage +52.34 KB
tag name (type) (pr)         802.34 KB - 1.29x memory usage +182.35 KB
tag name (type) (main)       849.46 KB - 1.37x memory usage +229.47 KB
class multiple (pr)          744.91 KB - 1.20x memory usage +124.92 KB
class multiple (main)        838.79 KB - 1.35x memory usage +218.80 KB

-- fprof before
%                                                           CNT           ACC           OWN
[{ "<0.99.0>",                                            28450,    undefined,       44.885}].   %%

{  {'Elixir.Floki.Finder',traverse_with,4},                6028,       34.169,       10.501}.
{  {'Elixir.Floki.HTMLTree',build_tree,5},                 3214,        7.492,        5.233}.
{  {'Elixir.Floki.Selector','match?',3},                   2009,       15.975,        4.550}.

-- fprof after
%                                                           CNT           ACC           OWN
[{ "<0.99.0>",                                            24373,    undefined,       42.737}].   %%

{  {'Elixir.Floki.Finder',traverse_with,3},                2011,       32.079,        6.780}.
{  {'Elixir.Floki.HTMLTree',build_tree,5},                 3214,        7.616,        5.340}.
{  {'Elixir.Floki.Selector','match?',3},                   2009,       18.178,        5.023}.

@philss
Copy link
Owner

philss commented Feb 9, 2024

@ypconstante would you mind to rebase and solve the conflict? Thanks!

@philss philss merged commit c2c1a25 into philss:main Feb 9, 2024
9 checks passed
@philss
Copy link
Owner

philss commented Feb 9, 2024

@ypconstante thank you! :D

@ypconstante ypconstante deleted the reduce-traverse-calls branch February 17, 2024 16:43
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