-
Notifications
You must be signed in to change notification settings - Fork 5.9k
[SOT] Guard dict itself in dict.get
#71223
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
[SOT] Guard dict itself in dict.get
#71223
Conversation
|
你的PR提交成功,感谢你对开源项目的贡献! |
dict.getdict.get
| # `d.get(key, default)` equivalent to `d[key] if key in d else default` | ||
| # We need guard `key in d`, but now we simply guard `d` and `key` separately | ||
| # (`key` is guarded in __getitem__ and key is guarded in getitem) | ||
| # TODO: We should add some tracker to record the key and the dict |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| # TODO: We should add some tracker to record the key and the dict | |
| # TODO: We should add some trackers to record the key and the dict |
🤪
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (2)
python/paddle/jit/sot/opcode_translator/executor/variables/container.py:885
- The current implementation of get() still lacks a guard for the kwargs case (i.e. when a default is provided). This may lead to unintended cache hits when different values for kwargs are supplied; consider adding a guard for kwargs as described in the PR details.
self.graph.add_global_guarded_variable(self)
test/sot/test_min_graph_size.py:117
- [nitpick] The new test for get_arg_from_kwargs checks that the function returns (None, None) for both calls, but it does not verify that the cache correctly distinguishes between different kwargs inputs. Consider extending this test with assertions that explicitly validate cache invalidation when kwargs differ.
self.assert_results(get_arg_from_kwargs, None, y=1)
dict.getdict.get
PR Category
Execute Infrastructure
PR Types
Bug fixes
Description
当
MIN_GRAPH_SIZE=10时,这里我们生成的字节码如下:这里连续 load 两次
x,这是因为x和y都是同一个对象None,是LOAD_CONST产生的,这里没什么问题但是生成的 guard 却只是
lambda frame: id(type(frame.f_locals['x'])) == 94068900165056 and frame.f_locals['x'] == None这里明显有一个问题,就是
y无论是什么都会命中 guard,而且这里生成的代码全是 loadx,因此后面当y传入和x不一样的值时就出问题了其实这里本质问题是缺失了
kwargs的 guard,这里d.get(key, default)等价于d[key] if key in d else default,这里控制流 condkey in d应该加到 guard 里,而这里我们是没有加的这里我们当然可以构造
key in d的 Variable 并将其 guard 住,但我们这里使用了一个简单的方式,直接 guard 住d和key,d是新加的,key在getitem时会自动 guard 住未来我们可以实现 polyfill 的 dispatch 机制,将
d.get(key, default)派发到d[key] if key in d else default,以自动记录这里的key in d到 guardPCard-66972