-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Performance improvements in core library #1683
Changes from all commits
5d2a7fa
2ddcce5
7e321e5
8283dc2
049a1ad
9d9ad7b
a23d3eb
e43d93b
85431d0
bdaebe4
d68c22e
1d2917e
26c52d7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,20 +27,25 @@ jobs: | |
- name: Install dependencies | ||
run: poetry install --with dev | ||
- name: Run benchmarks | ||
run: make benchmark | ||
run: OUTPUT=out/benchmark-baseline.json make -s benchmark | ||
- name: Upload benchmark baseline | ||
uses: actions/upload-artifact@v4 | ||
with: | ||
name: benchmark-baseline.json | ||
path: libs/langgraph/out/benchmark.json | ||
compare: | ||
name: benchmark-baseline | ||
path: libs/langgraph/out/benchmark-baseline.json | ||
benchmark: | ||
runs-on: ubuntu-latest | ||
defaults: | ||
run: | ||
working-directory: libs/langgraph | ||
needs: [baseline] | ||
steps: | ||
- uses: actions/checkout@v4 | ||
- id: files | ||
name: Get changed files | ||
uses: Ana06/[email protected] | ||
with: | ||
format: json | ||
- name: Set up Python 3.11 + Poetry ${{ env.POETRY_VERSION }} | ||
uses: "./.github/actions/poetry_setup" | ||
with: | ||
|
@@ -50,13 +55,36 @@ jobs: | |
- name: Install dependencies | ||
run: poetry install --with dev | ||
- name: Run benchmarks | ||
run: make benchmark | ||
id: benchmark | ||
run: | | ||
{ | ||
echo 'OUTPUT<<EOF' | ||
make -s benchmark | ||
echo EOF | ||
} >> "$GITHUB_OUTPUT" | ||
- name: Download benchmark baseline | ||
uses: actions/download-artifact@v4 | ||
with: | ||
name: benchmark-baseline.json | ||
path: libs/langgraph/out | ||
merge-multiple: true | ||
- name: Compare benchmarks | ||
run: poetry run pyperf compare_to out/benchmark-baseline.json out/benchmark.json --table --group-by-speed >> $GITHUB_OUTPUT | ||
id: compare | ||
run: | | ||
{ | ||
echo 'OUTPUT<<EOF' | ||
poetry run pyperf compare_to out/benchmark-baseline.json out/benchmark.json --table --group-by-speed | ||
echo EOF | ||
} >> "$GITHUB_OUTPUT" | ||
- name: Annotation | ||
run: echo "::notice file=libs/langgraph/bench/__main__.py::$GITHUB_OUTPUT" | ||
uses: actions/github-script@v7 | ||
with: | ||
script: | | ||
const file = JSON.parse(`${{ steps.files.outputs.added_modified_renamed }}`)[0] | ||
core.notice(`${{ steps.benchmark.outputs.OUTPUT }}`, { | ||
title: 'Benchmark results', | ||
file, | ||
}) | ||
core.notice(`${{ steps.compare.outputs.OUTPUT }}`, { | ||
title: 'Comparison against main', | ||
file, | ||
}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -46,6 +46,8 @@ | |
from langgraph.pregel.types import All, PregelExecutableTask, PregelTask | ||
from langgraph.utils.config import merge_configs, patch_config | ||
|
||
EMPTY_SEQ = tuple() | ||
|
||
|
||
class WritesProtocol(Protocol): | ||
name: str | ||
|
@@ -78,7 +80,10 @@ def should_interrupt( | |
task | ||
for task in tasks | ||
if ( | ||
(not task.config or TAG_HIDDEN not in task.config.get("tags")) | ||
( | ||
not task.config | ||
or TAG_HIDDEN not in task.config.get("tags", EMPTY_SEQ) | ||
) | ||
if interrupt_nodes == "*" | ||
else task.name in interrupt_nodes | ||
) | ||
|
@@ -182,11 +187,10 @@ def apply_writes( | |
for chan in task.triggers | ||
if chan not in RESERVED and chan in channels | ||
}: | ||
if channels[chan].consume(): | ||
if get_next_version is not None: | ||
checkpoint["channel_versions"][chan] = get_next_version( | ||
max_version, channels[chan] | ||
) | ||
if channels[chan].consume() and get_next_version is not None: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is this just stylistic or does it actually improve perf? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No this was just style |
||
checkpoint["channel_versions"][chan] = get_next_version( | ||
max_version, channels[chan] | ||
) | ||
|
||
# clear pending sends | ||
if checkpoint["pending_sends"]: | ||
|
@@ -216,8 +220,7 @@ def apply_writes( | |
updated_channels: set[str] = set() | ||
for chan, vals in pending_writes_by_channel.items(): | ||
if chan in channels: | ||
updated = channels[chan].update(vals) | ||
if updated and get_next_version is not None: | ||
if channels[chan].update(vals) and get_next_version is not None: | ||
checkpoint["channel_versions"][chan] = get_next_version( | ||
max_version, channels[chan] | ||
) | ||
|
@@ -370,6 +373,8 @@ def prepare_single_task( | |
proc = processes[packet.node] | ||
if node := proc.node: | ||
managed.replace_runtime_placeholders(step, packet.arg) | ||
if proc.metadata: | ||
metadata.update(proc.metadata) | ||
writes = deque() | ||
task_checkpoint_ns = f"{checkpoint_ns}:{task_id}" | ||
return PregelExecutableTask( | ||
|
@@ -379,9 +384,7 @@ def prepare_single_task( | |
writes, | ||
patch_config( | ||
merge_configs( | ||
config, | ||
processes[packet.node].config, | ||
{"metadata": metadata}, | ||
config, {"metadata": metadata, "tags": proc.tags} | ||
), | ||
run_name=packet.node, | ||
callbacks=( | ||
|
@@ -478,6 +481,8 @@ def prepare_single_task( | |
|
||
if for_execution: | ||
if node := proc.node: | ||
if proc.metadata: | ||
metadata.update(proc.metadata) | ||
writes = deque() | ||
task_checkpoint_ns = f"{checkpoint_ns}:{task_id}" | ||
return PregelExecutableTask( | ||
|
@@ -487,9 +492,7 @@ def prepare_single_task( | |
writes, | ||
patch_config( | ||
merge_configs( | ||
config, | ||
proc.config, | ||
{"metadata": metadata}, | ||
config, {"metadata": metadata, "tags": proc.tags} | ||
), | ||
run_name=name, | ||
callbacks=( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -97,6 +97,9 @@ def __radd__(self, other: dict[str, Any]) -> "AddableUpdatesDict": | |
raise TypeError("AddableUpdatesDict does not support right-side addition") | ||
|
||
|
||
EMPTY_SEQ = tuple() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we just make a single empty_seq in an internal _constants file or something There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess so? |
||
|
||
|
||
def map_output_updates( | ||
output_channels: Union[str, Sequence[str]], | ||
tasks: list[tuple[PregelExecutableTask, Sequence[tuple[str, Any]]]], | ||
|
@@ -106,7 +109,7 @@ def map_output_updates( | |
output_tasks = [ | ||
(t, ww) | ||
for t, ww in tasks | ||
if (not t.config or TAG_HIDDEN not in t.config.get("tags")) | ||
if (not t.config or TAG_HIDDEN not in t.config.get("tags", EMPTY_SEQ)) | ||
and ww[0][0] != ERROR | ||
and ww[0][0] != INTERRUPT | ||
] | ||
|
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.
Do you have an idea of how helpful this was? I guess this is sort of an inner loop
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.
This was more because the previous code was actually incorrect, as it could end up trying to do
<str> in None