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

[SR-9870] Dictionary needs an API suitable for memoization #52276

Open
dabrahams opened this issue Feb 6, 2019 · 7 comments
Open

[SR-9870] Dictionary needs an API suitable for memoization #52276

dabrahams opened this issue Feb 6, 2019 · 7 comments
Assignees
Labels
improvement standard library Area: Standard library umbrella

Comments

@dabrahams
Copy link
Collaborator

Previous ID SR-9870
Radar None
Original Reporter @dabrahams
Type Improvement

Attachment: Download

Additional Detail from JIRA
Votes 0
Component/s Standard Library
Labels Improvement
Assignee @lorentey
Priority Medium

md5: d2bb9e67d575089c4ebdf00102db57df

Issue Description:

We need an API like this:

extension Dictionary {
    /// Returns `self[key] ?? computeDefault()`, inserting the result of
    /// `computeDefault` into the dictionary if it is called.
    subscript(
        key: Key, setDefault computeDefault: @autoclosure ()->Value
    ) -> Value {
        @inline(__always)
        mutating get {
            if let r = self[key] { return r }
            let r = computeDefault()
            self[key] = r
            return r
        }
    }
}

There's no way to code it outside the standard library that avoids doing a 2nd dictionary lookup.

@belkadan
Copy link
Contributor

belkadan commented Feb 6, 2019

This works but doesn't really invalidate your request.

private func save<T>(_ x: inout T) -> T { return x}

extension Dictionary {
    subscript(
        key: Key, setDefault computeDefault: @autoclosure ()->Value
    ) -> Value {
        @inline(__always)
        mutating get {
            return save(&self[key, default: computeDefault()])
        }
    }
}

var x: [String: Int] = [:]
print(x["abc", setDefault: 1])
print(x["abc", setDefault: 2])
print(x)

@belkadan
Copy link
Contributor

belkadan commented Feb 6, 2019

cc @lorentey

@dabrahams
Copy link
Collaborator Author

@belkadan That is truly evil. Have to say I'm impressed!

@dabrahams
Copy link
Collaborator Author

Heh, but at least on cursory examination it generates even worse code than the other two ways I tried and doesn't avoid rehashing 🙁

my versions: x0.S x1.S

Jordan's evil version: x2.S

@lorentey
Copy link
Member

lorentey commented Feb 6, 2019

Yeah, this shouldn't need such contortions. A direct implementation would generate better code.

The good news is that Jordan's evil version only hashes twice on the slow path, when the insertion needs to resize the table. (In which case rehashing is necessary, as the dictionary switches to a new hash seed.)

Given that we already have a defaulting subscript with slightly different semantics, memoization would probably work better as a mutating method than a new subscript variant.

extension Dictionary {
  mutating func memoizeValue(forKey key: Key, _ computeValue: () -> Value) -> Value { ... }
}

var identifiers: [String: Int] = [:]
var nextId = 0
for string in ["Foo", "Bar", "Foo", "Foo", "Bar"] {
  let id = identifiers.memoizeValue(forKey: string) \{ let id = nextId; nextId += 1; return id }
  print(id)
}
// ⟹ 0 1 0 0 1

(I am not attached to this particular method name or signature, though.)

@dabrahams
Copy link
Collaborator Author

I just realized why I never thought of @belkadan's evil technique: the documentation for that subscript is wrong.

@lorentey: I have a bunch of other twisty non-memoization examples where I am currently forced to (ab)use that technique. I'll try to post links here when they are ready.

@dabrahams
Copy link
Collaborator Author

@lorentey here are the examples; look for “default:” in the code to find the places where the hack is used. I don't know if an API whose name involves “memoize” would really be appropriate for these uses.

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement standard library Area: Standard library umbrella
Projects
None yet
Development

No branches or pull requests

3 participants