Skip to content

Optimize EOF by reading types on demand#1034

Merged
chfast merged 1 commit into
ipsilon:masterfrom
elijahhampton:elijah/eof-dont-copy-type-section
Oct 14, 2024
Merged

Optimize EOF by reading types on demand#1034
chfast merged 1 commit into
ipsilon:masterfrom
elijahhampton:elijah/eof-dont-copy-type-section

Conversation

@elijahhampton

@elijahhampton elijahhampton commented Sep 27, 2024

Copy link
Copy Markdown
Contributor

Introduces EOF1Header::get_type function to read individual EOF
code types. This is fast procedure because we need to read at most 4
bytes and a know offset. By doing so we avoid allocating separated
vector to types.

Closes #1003.

@elijahhampton

Copy link
Copy Markdown
Contributor Author

This PR addresses: #1003

Comment thread lib/evmone/eof.cpp Outdated
Comment thread lib/evmone/eof.cpp Outdated
@chfast chfast added the EOF label Sep 28, 2024
@elijahhampton elijahhampton requested a review from chfast October 5, 2024 13:51
Comment thread lib/evmone/eof.hpp Outdated
Comment thread lib/evmone/eof.hpp Outdated
Comment thread lib/evmone/instructions.hpp Outdated
Comment thread lib/evmone/eof.cpp Outdated
Comment thread lib/evmone/eof.cpp Outdated
Comment thread lib/evmone/eof.cpp Outdated
Comment thread lib/evmone/eof.cpp Outdated
Comment thread lib/evmone/eof.cpp Outdated
Comment thread lib/evmone/eof.cpp Outdated
@elijahhampton elijahhampton requested a review from chfast October 8, 2024 19:32
Comment thread lib/evmone/eof.cpp Outdated
Comment thread lib/evmone/eof.hpp Outdated
Comment thread lib/evmone/eof.hpp
Comment thread lib/evmone/eof.hpp Outdated
Comment thread lib/evmone/eof.cpp Outdated
Comment thread lib/evmone/eof.cpp Outdated
if (type_section_size != code_sizes.size() * 4)
return EOFValidationError::invalid_type_section_size;

auto validation_error = validate_types(container, type_section_offset, type_section_size);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is quite an issue: validate_type() should not be used in validate_header() and invoke separately after validate_header(). This way we can just pass EOF1Header to validate_types() and use its .get_type() method.

But maybe this requires a separate PR. What do you think @gumb0?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sounds good to try in a separate PR.

It made sense to do it here before, becase "validate header" really meant "validate header and types and return them togethere in one struct"

@chfast chfast requested a review from gumb0 October 9, 2024 19:12
Comment thread lib/evmone/eof.hpp Outdated
uint8_t version = 0;

/// Size of every type section.
uint16_t type_section_size = 0;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We don't need to store the size at all, because header validation guarantees that type_section_size == code_sizes.siz() * 4

@elijahhampton elijahhampton requested a review from chfast October 12, 2024 16:05
@chfast chfast changed the title Optimizes validate_types to read EOF types on demand. Optimize EOF by reading types on demand Oct 14, 2024
Introduces `EOF1Header::get_type` function to read individual EOF
code types. This is fast procedure because we need to read at most 4
bytes and a know offset. By doing so we avoid allocating separated
vector to types.
@chfast chfast enabled auto-merge (squash) October 14, 2024 14:58
@chfast chfast merged commit 589c1d6 into ipsilon:master Oct 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

EOF: Don't copy type section

3 participants