Skip to content

Conversation

@lambdageek
Copy link
Member

@lambdageek lambdageek commented Jan 27, 2021

  • use prepend instead of append
    g_list_append has to repeatedly traverse the list using g_list_last to find the last element.

    Instead use g_list_prepend to add the new bb to the front of the list and then reverse at the end (g_list_reverse is inplace and doesn't allocate)

  • don't make the list at all unless it's needed.
    it's only needed when generating sequence points.

g_list_append has to repeatedly traverse the list using g_list_last to
find the last element.

Instead use g_list_prepend to add the new bb to the front of the list
and then reverse at the end (g_list_reverse is inplace and doesn't allocate)
@ghost
Copy link

ghost commented Jan 27, 2021

Tagging subscribers to this area: @BrzVlad
See info in area-owners.md if you want to be subscribed.

Issue Details

g_list_append has to repeatedly traverse the list using g_list_last to
find the last element.

Instead use g_list_prepend to add the new bb to the front of the list
and then reverse at the end (g_list_reverse is inplace and doesn't allocate)

Author: lambdageek
Assignees: -
Labels:

area-Codegen-Interpreter-mono

Milestone: -

@lambdageek
Copy link
Member Author

Saw this show up kind of hot in a microbenchmark profile.

Copy link
Member

@BrzVlad BrzVlad left a comment

Choose a reason for hiding this comment

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

An even better solution would be to not create this list at all when not generating seq points.

Still fill in offset_to_bb, but don't make a list unless needed.
@lambdageek lambdageek changed the title [interp] Add basic blocks in reverse order, then reverse [interp] basic_blocks list improvements Jan 27, 2021
@lambdageek
Copy link
Member Author

@BrzVlad updated


td->basic_blocks = g_list_append_mempool (td->mempool, td->basic_blocks, bb);
/* Add the blocks in reverse order */
if (make_list)
Copy link
Member

Choose a reason for hiding this comment

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

any reason for not using td->gen_sdb_seq_points directly here ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not really. Wasn't sure if there's other reasons why we might need a list of blocks.

@lambdageek lambdageek merged commit 8ec7db1 into dotnet:master Jan 28, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Feb 27, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants