jameysharp commented on issue #4536:
Very cool!
If I'm not mis-reading things,
Block
does not need to be dropped (it's just a newtype for u32 and it doesn't implementDrop
), soHashSet::clear
is constant-time. In that case, how about just clearing the set before using it, where you're currentlydebug_assert
ing that it's empty, and removing all the magic drop-guard machinery?
jameysharp commented on issue #4536:
Also,
can_optimize_var_lookup
is only called fromuse_var_nonlocal
, which has a&mut self
. So you could changecan_optimize_var_lookup
to also take&mut self
and then get rid of theRefCell
.
fitzgen commented on issue #4536:
Also,
can_optimize_var_lookup
is only called fromuse_var_nonlocal
, which has a&mut self
. So you could changecan_optimize_var_lookup
to also take&mut self
and then get rid of theRefCell
.Nice, I didn't notice that!
fitzgen commented on issue #4536:
If I'm not mis-reading things,
Block
does not need to be dropped (it's just a newtype for u32 and it doesn't implementDrop
), soHashSet::clear
is constant-time. In that case, how about just clearing the set before using it, where you're currentlydebug_assert
ing that it's empty, and removing all the magic drop-guard machinery?The idea was to empty the set as soon as it isn't used anymore, rather than at the last minute before use, but I guess there isn't really a good reason for that since it doesn't cut down on peak memory usage (which is kind of the whole point of this PR!)
jameysharp commented on issue #4536:
Yeah, rather than saying "clear is constant-time", I should have said that it doesn't do anything except mark the table as empty. So it doesn't matter when you do it, as long as nobody else looks at it.
Last updated: Dec 23 2024 at 12:05 UTC