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

refactor: minimize pointers #209

Merged
merged 61 commits into from
Mar 7, 2024
Merged

refactor: minimize pointers #209

merged 61 commits into from
Mar 7, 2024

Conversation

aleksasiriski
Copy link
Member

@aleksasiriski aleksasiriski commented Feb 9, 2024

Idea:
Avoiding to use pointers results in having less garbage collection (most vars are on stack) and makes the code more predictable (if something changes, it must be returned). Of course, not everything should be passed by value and not every pointer is heap allocated so some meaningful situations remain where a pointer is being used (relay, colly, retrievedResult).

Tips:

  • avoid returning pointers, they will be heap allocated
  • passing pointers down (func1->func2->func3, w/o returning them) is stack allocated
  • passing pointers leads to possible changes in the data, whereas copy-by-value assures no changes in the func that passed it
  • most structs are small and the big parts in them are usually pointers and not actual big data structs, so for most usecases passing copy-by-value vs pointer's performance penalty is negligible

Changes:

  • Refactored colly to use less pointers, moving &params into returns
  • Moved context to be the first param in functions (except if a structs is passed because it gets used/modified, than that is the first param)
  • Made errors not be set via single retError pointer but returned and aggregated into an error slice
  • Refactor cli flags, cache, config, search and bucket to avoid pointers
  • Refactor rank and result to have better file structure and avoid pointers
  • Refactor engines to have init collector func setup both col and pagesCol collector
  • Refactor every file that uses config struct to accept only needed lesser structs as params

Off topic changes:

  • Switch go.mod to require go1.22 and switched for range loops to use _, var syntax since it doesn't retain the same adress anymore

@aleksasiriski aleksasiriski marked this pull request as draft February 9, 2024 21:30
…versal and switched to using retError channel for requests
@aleksasiriski aleksasiriski changed the title refactor: Pointers refactor: remove pointers Feb 9, 2024
@aleksasiriski aleksasiriski marked this pull request as ready for review February 10, 2024 21:58
Base automatically changed from feat-image-search to main March 1, 2024 18:24
@aleksasiriski aleksasiriski requested a review from k4lizen March 3, 2024 19:45
src/config/structs.go Outdated Show resolved Hide resolved
src/search/engines/_sedefaults/colly.go Outdated Show resolved Hide resolved
src/search/engines/_sedefaults/colly.go Show resolved Hide resolved
src/search/engines/bing/bing.go Outdated Show resolved Hide resolved
@aleksasiriski aleksasiriski requested a review from k4lizen March 7, 2024 14:55
Copy link
Member

@k4lizen k4lizen left a comment

Choose a reason for hiding this comment

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

LGTM
GJ

@aleksasiriski aleksasiriski merged commit 5390d4e into main Mar 7, 2024
5 of 6 checks passed
@aleksasiriski aleksasiriski deleted the refactor-pointers branch March 7, 2024 17:57
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