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 ZeroDictionary struct #475

Merged
merged 7 commits into from
Jun 28, 2024
Merged

Conversation

TAdev0
Copy link
Member

@TAdev0 TAdev0 commented Jun 20, 2024

This PR modifies the fields of ZeroDictionary struct to have only pointers . Because GetDictionary actually creates a copy of the dict :

// Given a memory address, it looks for the right dictionary using the segment index. If no
// segment is associated with the given segment index, it errors
func (dm *ZeroDictionaryManager) GetDictionary(dictAddr mem.MemoryAddress) (ZeroDictionary, error) {
	dict, ok := dm.Dictionaries[dictAddr.SegmentIndex]
	if ok {
		return dict, nil
	}
	return ZeroDictionary{}, fmt.Errorf("no dictionary at address: %s", dictAddr)
}

we will gain in performance in storing only an address for the whole mapping in the ZeroDictionary

@TAdev0 TAdev0 self-assigned this Jun 20, 2024
@TAdev0 TAdev0 requested review from har777 and cicr99 and removed request for har777 June 20, 2024 21:27
@TAdev0
Copy link
Member Author

TAdev0 commented Jun 20, 2024

@har777 we discussed the fact that FreeOffset field is of type *uint64 , its actually required with our current impl of GetDictionary , as we return a copy. If i just change it for a uint64 it doesnt work anymore and any offset incrementation is done on a local copy

Sh0g0-1758
Sh0g0-1758 approved these changes Jun 28, 2024
@TAdev0 TAdev0 merged commit d56008d into NethermindEth:main Jun 28, 2024
4 checks passed
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.

None yet

2 participants