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

Add Features: add MAdd and MGet #77

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

bjkxt
Copy link

@bjkxt bjkxt commented Nov 24, 2020

Update lru.go. Simple functions for many keys' operations with less mutex race.

add MAdd and MGet
Copy link
Member

@jefferai jefferai left a comment

Choose a reason for hiding this comment

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

Looks good to me, going to rope one more person into reviewing.

@briankassouf can you just 👀 real quick? Mostly in case this is something you think you might have immediate uses for :-)

@jefferai
Copy link
Member

jefferai commented Jan 4, 2021

A change just went in that updates how eviction is handled -- can you update this PR against it?

@jefferai
Copy link
Member

jefferai commented Feb 1, 2021

@bjkxt Got an approval from Brian, just looking for your changes against the new eviction code I think!

@bjkxt
Copy link
Author

bjkxt commented Mar 9, 2021

@bjkxt Got an approval from Brian, just looking for your changes against the new eviction code I think!

I think MAdd just takes the evictions of input keys and values to the caller. And the caller can decide how to handle these evictions.

@jefferai
Copy link
Member

jefferai commented Mar 9, 2021

Two issues:

  • There seem to be no tests for this functionality
  • The code doesn't actually check whether the various slices are the same length, so it will panic if the caller provides bad input

@hashicorp-cla
Copy link

hashicorp-cla commented Sep 9, 2021

CLA assistant check
All committers have signed the CLA.

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.

4 participants