Skip to content

Improve module accessors performance#7

Closed
zhouwfang wants to merge 15 commits into
fill-side-tablefrom
apply-new-side-table
Closed

Improve module accessors performance#7
zhouwfang wants to merge 15 commits into
fill-side-tablefrom
apply-new-side-table

Conversation

@zhouwfang

Copy link
Copy Markdown
Owner

@zhouwfang zhouwfang changed the title Improve module accessors Improve module accessors performance Jan 15, 2025
}
let saved = parser.save();
let entry = &self.side_table[x as usize];
let range = &entry.parser_range;

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

This has a bug. I have some clue, and you may help clarify the confusion.

The parsing for the code section seemed (before my changes) different between here and in validation. In validation, there is a step of parsing locals embedded in the for loop of functions. But in execution, appending locals is done after (in particular, outside) this Module::func(), e.g. here. The confusion is the parser layouts are different for the code section. Due to the parsing locals step in validation, maybe we need to subtract the length of the locals?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

First, we should probably store core_start in the side table. It's only 4 bytes and saves the loop in Module::section().

Regarding the bug, indeed we should move the let parser_start right after the split_at in valid.rs, because that's what this function was doing.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

By core_start, do you mean code_start?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

oops sorry yes

@zhouwfang
zhouwfang force-pushed the apply-new-side-table branch from 38ad38d to 28d29a5 Compare January 16, 2025 05:51
@zhouwfang

Copy link
Copy Markdown
Owner Author

Will continue in google#723

@zhouwfang zhouwfang closed this Jan 16, 2025
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