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

Encapsulated encoders #968

Closed
2 tasks done
wlandau opened this issue Aug 1, 2019 · 6 comments
Closed
2 tasks done

Encapsulated encoders #968

wlandau opened this issue Aug 1, 2019 · 6 comments

Comments

@wlandau
Copy link
Member

wlandau commented Aug 1, 2019

Prework

Description

We have 4 hash tables to encode and decode paths and namespaced functions.

  • ht_encode_path
  • ht_decode_path
  • ht_encode_namespaced
  • ht_decode_namespaced

Let's add some OOP. We could create a new "encoder" class that inherits from the hash table class. Depends on #967.

@wlandau
Copy link
Member Author

wlandau commented Sep 13, 2019

Let's use object composition instead of inheritance.

@wlandau
Copy link
Member Author

wlandau commented Sep 13, 2019

Idea:

  • Create an encoder class to encode and decode stuff.
  • Create an encoder_path with ht_encode_path and ht_decode_path().
  • Create another for namespaces.
  • Maybe also deal with progress_hashmap this way too.
  • Group these in an encoders field of config.

@wlandau
Copy link
Member Author

wlandau commented Sep 13, 2019

Nope, let's just make encoding and decoding part of the decorated storr.

@wlandau
Copy link
Member Author

wlandau commented Sep 13, 2019

Yup, extra functions in the decorated storr capture all of this. It completes the encapsulation, moving from exposed hash tables to reference class methods that use them under the hood.

@wlandau
Copy link
Member Author

wlandau commented Sep 13, 2019

Just need to verify that #1011 did not reduce performance.

@wlandau
Copy link
Member Author

wlandau commented Sep 13, 2019

Just ran a couple versions of the overhead example. Flame graphs look the same as before with no obvious bottlenecks.

@wlandau wlandau closed this as completed Sep 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant