GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — perf: Key registry proto caches on version_id so a refresh does not rebuild objects by chandlerok · Pull Request #7073 · feast-dev/feast

Conversation

@chandlerok

@chandlerok chandlerok commented Oct 26, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

What this PR does / why we need it:

The registry_proto_cache decorators in proto_registry_utils keyed on id(registry_proto) as well as version_id. A registry refresh parses a new proto object, so every refresh missed the cache and rebuilt every object from proto even when the registry had not changed. With 14 feature services that rebuild is about 35 ms per refresh, most of it typeguard checks in Field.__init__. Measured locally with a file registry, refresh plus list_feature_services(allow_cache=True):

per refresh
before 34.9 ms
after 0.1 ms

Changes:

  • Key the three cache decorators on version_id alone. A registry without a version_id falls back to object identity, which is today's behavior.
  • Cache get_feature_service per version with the existing registry_proto_cache_name decorator. get_online_features calls it on every request and it rebuilt the FeatureService from proto each time, about 2 ms per call.
  • Stamp SqlRegistry.proto() and SnowflakeRegistry.proto() with a version_id. Neither set one, so for those registries the caches would have kept falling back to object identity and missing on every refresh. The value is the sha1 of the deterministic serialization, stable across refreshes and different after any apply or delete. On a 421 KB proto the hash costs 0.2 ms. The file, S3, GCS, Azure, and HDFS stores already stamp a uuid on each write.

Profiled effect on get_online_features with a file registry, 100 entities, 4 feature views: a warm read has no from_proto or typeguard frames left on the registry side. The first read after a refresh costs 14.2 ms against 19.6 ms with the old object-identity key, and the same 5 ms saving now applies to SQL-registry deployments once per cache refresh.

No behavior change for callers. Cached objects were already shared between calls on the same proto object.

Which issue(s) this PR fixes:

Refs #7072 (proposal 3)

Checks

  • I've made sure the tests are passing.
  • My commits are signed off (git commit -s)
  • My PR title follows conventional commits format

Testing Strategy

  • Unit tests
  • Integration tests
  • Manual tests
  • Testing is not required for this change

Unit tests cover a re-parsed proto with the same version_id hitting the cache, a new version_id missing it, registries without a version_id staying per-object, get_feature_service returning the cached object, and a SQL registry whose version_id is stable across proto() calls and refreshes and changes after an apply. sdk/python/tests/unit/infra/registry passes locally (164 tests; three modules skip collection for missing optional dependencies).

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@chandlerok @claude
perf: Key registry proto caches on version_id so a refresh does not r…
7deb605
…ebuild objects

The registry_proto_cache decorators keyed on id(registry_proto) as well as
version_id. Every registry refresh parses a new proto object, so each refresh
missed the cache and rebuilt every object from proto even when the registry had
not changed. For a registry with 14 feature services that rebuild costs about
35 ms, most of it typeguard checks in Field.__init__.

Key on version_id alone, falling back to the object identity only when a
registry does not set a version_id. Also cache get_feature_service per
version, which get_online_features calls on every request.

Refs feast-dev#7072

Co-Authored-By: Claude Fable 5.1 <[email protected]>
Signed-off-by: Chandler King <[email protected]>
@chandlerok
chandlerok requested a review from a team as a code owner October 26, 2026 22:38
@chandlerok chandlerok mentioned this pull request Oct 26, 2026
Online read: columnar store return and proto-direct to_arrow to cut client CPU on wide reads #7072
Open
@codecov-commenter

codecov-commenter commented Oct 26, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 88.88889% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 51.00%. Comparing base (1e5e90c) to head (a74e783).

Files with missing lines Patch % Lines
...ython/feast/infra/registry/proto_registry_utils.py 80.00% 1 Missing ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #7073      +/-   ##
==========================================
+ Coverage   50.97%   51.00%   +0.03%     
==========================================
  Files         455      455              
  Lines       56994    57001       +7     
  Branches     8321     8321              
==========================================
+ Hits        29055    29076      +21     
+ Misses      25954    25940      -14     
  Partials     1985     1985              
Flag Coverage Δ
go-feature-server 30.58% <ø> (+0.20%) ⬆️
python-unit 52.47% <88.88%> (+0.01%) ⬆️
Files with missing lines Coverage Δ
sdk/python/feast/infra/registry/snowflake.py 39.25% <100.00%> (+0.21%) ⬆️
sdk/python/feast/infra/registry/sql.py 61.43% <100.00%> (+0.38%) ⬆️
...ython/feast/infra/registry/proto_registry_utils.py 75.49% <80.00%> (+0.29%) ⬆️

... and 3 files with indirect coverage changes


Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1e5e90c...a74e783. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@chandlerok @claude
perf: Stamp SQL and Snowflake registry protos with a content-hash ver…
a74e783
…sion_id

SqlRegistry.proto() and SnowflakeRegistry.proto() never set version_id, so the
version-keyed caches fell back to object identity and every cache refresh
still rebuilt every object from proto. Set version_id to the sha1 of the
deterministic serialization, which is stable across refreshes and changes on
any apply or delete. On a 421 KB registry proto the hash costs 0.2 ms.

Refs feast-dev#7072

Co-Authored-By: Claude Fable 5.1 <[email protected]>
Signed-off-by: Chandler King <[email protected]>
@chandlerok

chandlerok commented Oct 26, 2026

Copy link
Copy Markdown
Contributor Author

@ntkathole this follows on from #6870 and #7072. Two small commits: version-keyed registry caches, and a version_id on the SQL and Snowflake registry protos so those registries get the same cache hits. Numbers and profile are in the description.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

1 similar comment
@chandlerok

chandlerok commented Oct 26, 2026

Copy link
Copy Markdown
Contributor Author

@ntkathole this follows on from #6870 and #7072. Two small commits: version-keyed registry caches, and a version_id on the SQL and Snowflake registry protos so those registries get the same cache hits. Numbers and profile are in the description.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

shuchu
shuchu approved these changes Nov 4, 2026
View reviewed changes

@shuchu shuchu left a comment

Copy link
Copy Markdown
Collaborator

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.

lgtm

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@shuchu

shuchu commented Nov 4, 2026

Copy link
Copy Markdown
Collaborator

@chandlerok the PR is out-of-date, please update it.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@shuchu

shuchu commented Nov 10, 2026

Copy link
Copy Markdown
Collaborator

@copilot resolve the merge conflicts in this pull request

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@shuchu
Merge branch 'master' into perf/online-response-arrow-and-registry-cache
87deb2f
@shuchu

shuchu commented Nov 10, 2026

Copy link
Copy Markdown
Collaborator

my bad, the conflicts was not solved correctly.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@shuchu shuchu shuchu approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@chandlerok @codecov-commenter @shuchu
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.