Skip to content

Commit

Permalink
[perf fix] Add manual GC points to 'if' and 'case'
Browse files Browse the repository at this point in the history
This resolves issue #2123.

The 'filter' loop would evaluate expressions, and allocate, without
every hitting a GC point.

To break out of a loop or recursion, you will need 'if' or 'case'.  So
I believe this should work everywhere, just as it does the
test/bug-2123.* repro.

I'm not quite sure why the bug depended on get() NOT having a default
arg though.  Could look into that more.
  • Loading branch information
Andy C committed Nov 17, 2024
1 parent b52392e commit 0634190
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 2 deletions.
8 changes: 8 additions & 0 deletions osh/cmd_eval.py
Original file line number Diff line number Diff line change
Expand Up @@ -1726,6 +1726,10 @@ def _Dispatch(self, node, cmd_st):
# It will be taken care of by command.Simple, condition, etc.
status = self._DoIf(node)

# Perf bug fix: loops might only execute 'if', but we still
# need to GC
self._LeafTick()

elif case(command_e.Case):
node = cast(command.Case, UP_node)

Expand All @@ -1734,6 +1738,10 @@ def _Dispatch(self, node, cmd_st):
self._MaybeRunDebugTrap()
status = self._DoCase(node)

# Perf bug fix: loops might only execute 'case', but we still
# need to GC
self._LeafTick()

elif case(command_e.WhileUntil):
node = cast(command.WhileUntil, UP_node)

Expand Down
5 changes: 3 additions & 2 deletions test/bug-2123.sh
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,12 @@ demo() {
local ysh=_bin/cxx-opt/ysh
ninja $ysh

#OILS_GC_STATS=1 $ysh test/bug-2123.ysh
#OILS_GC_STATS=1 _OILS_GC_VERBOSE=1 $ysh test/bug-2123.ysh
time OILS_GC_STATS=1 $ysh test/bug-2123.ysh

# max RSS
# 246
/usr/bin/time --format '%e %M' $ysh test/bug-2123.ysh
# /usr/bin/time --format '%e %M' $ysh test/bug-2123.ysh
}

debug() {
Expand Down
8 changes: 8 additions & 0 deletions test/bug-2123.ysh
Original file line number Diff line number Diff line change
@@ -1,10 +1,15 @@
# Run with _bin/cxx-opt/ysh

proc filter (; predicate) {
#var i = 0
for line in (io.stdin) {
if (io->evalExpr(predicate, vars={_val: fromJson8(line)})) {
write -- $line
}
#setvar i += 1
#if (i % 1000 === 0) {
# echo "i $i"
#}
}
}

Expand Down Expand Up @@ -67,7 +72,10 @@ proc without-default {
} < $f

#time filter [get(_val, 'missing-key', 0) === 0] < $f >/dev/null

echo '****** BEGIN filter'
time filter [get(_val, 'missing-key')] < $f #>/dev/null
echo '****** END filter'

write -- 'AFTER -------------------------------------------------------'

Expand Down

0 comments on commit 0634190

Please sign in to comment.