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

Improve module accessors performance #7

Closed
wants to merge 15 commits into from

Conversation

zhouwfang
Copy link
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
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

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
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

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
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