#development
Title
w
witty-crayon-22786
03/14/2020, 11:59 PMso, it looks like https://github.com/pantsbuild/pants/pull/9181 and https://github.com/pantsbuild/pants/pull/9166 are a significant regression: around 3x for
./pants filter ::x.svg
the
__contains__i'll look at this for a bit today, but it looks like it would also be a relatively easy thing to revert out of 1.26.x and follow up on.
👍 1
h
hundreds-father-404
03/15/2020, 12:23 AMRevert sounds fine for me in 1.26.x. For 1.27.x, we can redesign the implementation while keeping the same API
w
witty-crayon-22786
03/15/2020, 12:24 AMgoing to spend a bit more time on it today... if i run into a wall will go for the revert.
👍 1
h
hundreds-father-404
03/15/2020, 12:26 AMCheck out the Ordered Set library. I think the best approach is to go back to having a list and a dict. But, I’m not sure how that would work with FOS because dicts are mutable :/
We use contains a lot in the target API, so I think getting that back down to O(1) is worth the extra space complexity
w
witty-crayon-22786
03/15/2020, 12:26 AMso... i'm not sure why they are.
...not hashable.
h
hundreds-father-404
03/15/2020, 12:27 AMDicts are mutable so not safe to hash
w
witty-crayon-22786
03/15/2020, 12:27 AM...oh. actually, that's clear. maybe. but not for the set.
ehh, if you don't expose any of the methods.
h
hundreds-father-404
03/15/2020, 12:28 AMMaybe you could use frozenset + tuple? Frozenset gives cheap lookups, tuple maintains order
w
witty-crayon-22786
03/15/2020, 12:28 AMgiving it a swing with just the dict. because "hash" is "iterate through all keys and hash them".
👍 1
and the frozenness is just a matter of whether you expose those methods.
h
hundreds-father-404
03/15/2020, 12:33 AMOh is the idea to solely use dict? That’s a great idea. Even if you have to override hash, that would be perfect. And you don’t need any complementary data type because dicts are ordered in 3.6+
w
witty-crayon-22786
03/15/2020, 12:33 AMyep
💯 1
h
hundreds-father-404
03/15/2020, 12:34 AMAwesome. Then you could use an identical implementation between OS and FOS. But, we only want to implement hash for FOS. It should eagerly fail when trying to use OS with the engine because it’s not hahable
w
witty-crayon-22786
03/15/2020, 12:34 AMyea