Skip to content

proposal: built-in map: add delete hook func And set hook func #60311

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

Closed
someview opened this issue May 19, 2023 · 8 comments
Closed

proposal: built-in map: add delete hook func And set hook func #60311

someview opened this issue May 19, 2023 · 8 comments

Comments

@someview
Copy link

#47649

// Package maps defines various functions useful with maps of any type.
package maps

// Keys returns the keys of the map m.
// The keys will be an indeterminate order.
func Keys[M constraints.Map[K, V], K comparable, V any](m M) []K

// Values returns the values of the map m.
// The values will be in an indeterminate order.
func Values[M constraints.Map[K, V], K comparable, V any](m M) []V

// Equal reports whether two maps contain the same key/value pairs.
// Values are compared using ==.
func Equal[M1, M2 constraints.Map[K, V], K, V comparable](m1 M1, m2 M2) bool

// EqualFunc is like Equal, but compares values using cmp.
// Keys are still compared with ==.
func EqualFunc[M1 constraints.Map[K, V1], M2 constraints.Map[K, V2], K comparable, V1, V2 any](m1 m1, m2 M2, cmp func(V1, V2) bool) bool

// Clear removes all entries from m, leaving it empty.
func Clear[M constraints.Map[K, V], K comparable, V any](m M)

// Clone returns a copy of m.  This is a shallow clone:
// the new keys and values are set using ordinary assignment.
func Clone[M constraints.Map[K, V], K comparable, V any](m M) M

// Copy copies all key/value pairs in src adding them to dst.
// When a key in src is already present in dst,
// the value in dst will be overwritten by the value associated
// with the key in src.
func Copy[M constraints.Map[K, V], K comparable, V any](dst, src M)

// PreDelete deletes any key/value pairs from m for which del returns true.
func PreDelete[M constraints.Map[K, V], K comparable, V any](k K, del func(K, V) bool)

//PostDelete deletes a key/value pairs, if oldvalue exists,  post will be called
func PostDelete[M constraints.Map[K, V], K comparable, V any](k K, post func((oldValue V))

// PostSet sets a key/value pair, if oldvalue exists,  post will be called
func PostSet[M constraints.Map[K, V], K comparable, V any](k K,post func((oldValue V))

some cases:

  • when delete a key,want to know if kvpairs exists exec some op
  • when set a key, want to know if kvpairs exists and exec some op

eg:
when cache evited some item, delete key from map want to do some op: only kvpairs exists,
set a kvpairs, we want to get the old kvpairs to eslimate cost,and using the new cost - the old cost

@someview someview changed the title generic maps: offer Upsert And Delete method that offer information for the key generic maps: offer PostDelete And PostSet method that offer information for the key May 19, 2023
@heschi
Copy link
Contributor

heschi commented May 19, 2023

cc @ianlancetaylor

@heschi heschi added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label May 19, 2023
@heschi heschi added this to the Backlog milestone May 19, 2023
@heschi heschi changed the title generic maps: offer PostDelete And PostSet method that offer information for the key maps: offer PostDelete And PostSet method that offer information for the key May 19, 2023
@ianlancetaylor
Copy link
Contributor

Most of these functions already exist in the upcoming 1.21 release. Can you trim this back to exactly what you are proposing? It would also help to point to some existing code that could use these new functions. Thanks.

@ianlancetaylor ianlancetaylor added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label May 20, 2023
@someview
Copy link
Author

someview commented May 20, 2023

the proposal: offer hook to stuff that cares about oldkvpairs when exec map operation

// PostDelete deletes a key/value pairs, if oldvalue exists,  post will be called
func PostDelete[M constraints.Map[K, V], K comparable, V any](k K, post func((oldValue V))

// PostSet sets a key/value pair, if oldvalue exists,  post will be called
func PostSet[M constraints.Map[K, V], K comparable, V any](k K,post func((oldValue V))

orginially ,we must write some code like this:

func doStuff(){
}
// delete a real key and do some stuff
// like evict action, offer observer to old kvpairs
var info= make(map[string]string,10)
func RemoveInfo(key string){
     if _,ok:= info[key];ok {
          delete(info,key)
         doStuff()
    }
}
// set a kvpairs  and dosome stuff  if oldkvpairs exists
//  like swap operation
var info= make(map[string]string,10)
func UpsertInfo(key ,val string){
     oldValue,ok:= info[key]
     info[key] = val
    if ok {
        doStuff()
     }
    }
}

In any cases which care about prev kvpairs, when we do some operation,such as set,delete.
Now the bove code would exec querycode twice. However, once is ok
scenario:

  • cache evicted and set
  • dboperation with kepairs in a map

@someview someview changed the title maps: offer PostDelete And PostSet method that offer information for the key maps: offer PostDelete And PostSet method that offer information for the prev kvPairs May 20, 2023
@ianlancetaylor
Copy link
Contributor

To be clear, adding this to the maps package won't be any more efficient than doing it yourself. So this doesn't have to be in the standard library. Can you point to existing code that would benefit from these functions? If possible, don't just describe it, actually show us the existing repository. Thanks.

Also, see #41130.

@ianlancetaylor ianlancetaylor changed the title maps: offer PostDelete And PostSet method that offer information for the prev kvPairs proposal: maps: add PostDelete And PostSet May 20, 2023
@ianlancetaylor ianlancetaylor removed WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels May 20, 2023
@ianlancetaylor ianlancetaylor modified the milestones: Backlog, Proposal May 20, 2023
@someview
Copy link
Author

someview commented May 22, 2023

To be clear, adding this to the maps package won't be any more efficient than doing it yourself. So this doesn't have to be in the standard library. Can you point to existing code that would benefit from these functions? If possible, don't just describe it, actually show us the existing repository. Thanks.

Also, see #41130.

Yeah,I‘m confused with the builtin map.This may be built-in,not in maps package.
#43113 has already mentioned similar problem。Because it's a private repo,I can not show it.But there is a public repo to do some thing like this: cache

// https://github.com/dgraph-io/ristretto/blob/main/policy.go#L145
when evicted a key in cache, calcuclate the cost of the key if the key exist, and do some stuff about the kvpairs
when set a key, if  old kvpairs with the key, decrease the total cost and do some stuff about the prev kvpairs

code:

func doStuff(){
}
// delete a real key and do some stuff
// like evict action, offer observer to old kvpairs
var info= make(map[string]string,10)
func RemoveInfo(key string){
     if _,ok:= info[key];ok {
          delete(info,key)
         doStuff()
    }
}
// set a kvpairs  and dosome stuff  if oldkvpairs exists
//  like swap operation
var info= make(map[string]string,10)
func UpsertInfo(key ,val string){
 //oldValue,ok:= info[key]
  //  info[key] = val
  //  if ok {
  //   doStuff()
  // }
    info.set(key,val,func(val))  // cb is called if the old val exist,just query once and atomic operation
}

func RemoveInfo(key string){
    // if _,ok:= info[key];ok {
    //      delete(info,key)
    //     doStuff()
    info.remove(key, func(val))  // cb is called if the old val exist,just query once and atomic operation
    }
}

@ianlancetaylor
Copy link
Contributor

I think we ruled out having a builtin in #41130, as discussed there. We would only reconsider that decision if there is new information.

@someview
Copy link
Author

I think we ruled out having a builtin in #41130, as discussed there. We would only reconsider that decision if there is new information.
This is not unique. This would be base operation for map:

  • iter
  • get
  • set
  • upsert -> return old val or no return value
  • delete->return old val or no return value

We can find similar api in other languages:

//c++ map
insertorassigin
// c# dict
addorupdate

In actually, this can be v,ok:=map[key] or v= map[key] pattern to Include all basic map operation

@someview someview changed the title proposal: maps: add PostDelete And PostSet proposal: built-in map: add delete hook func And set hook func May 24, 2023
@ianlancetaylor
Copy link
Contributor

I'm sorry, I don't see any new information here that would lead us to reconsider the decision made in #41130. Closing. Please comment if you disagree.

@ianlancetaylor ianlancetaylor closed this as not planned Won't fix, can't repro, duplicate, stale May 24, 2023
@golang golang locked and limited conversation to collaborators May 23, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants