From 95d1b1ba57050496a48bc6f22dbf8db00edd5e82 Mon Sep 17 00:00:00 2001 From: Test Date: Fri, 18 Oct 2024 16:49:30 -0400 Subject: [PATCH 1/3] build fix for lagrange basis performance --- src/bindings | 2 +- src/mina | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/bindings b/src/bindings index 1bb51bcb7..cba94257d 160000 --- a/src/bindings +++ b/src/bindings @@ -1 +1 @@ -Subproject commit 1bb51bcb7c1667e885a0d8d859b10b369708cd35 +Subproject commit cba94257dbccfbef41a63df85ed1a6cd43d3bd29 diff --git a/src/mina b/src/mina index 573f6c8f6..63725a490 160000 --- a/src/mina +++ b/src/mina @@ -1 +1 @@ -Subproject commit 573f6c8f6dcbedc16009b0ce985710bf5382c8c4 +Subproject commit 63725a4909a8053371f95961f7b55c61cb13d763 From fa3e1ef0bdde2960fc8ddf0334e26ebb77b1fe4a Mon Sep 17 00:00:00 2001 From: Test Date: Sat, 19 Oct 2024 09:58:39 -0400 Subject: [PATCH 2/3] adding a unit test --- src/lib/proof-system/cache.ts | 12 +++- .../cached-lagrange-basis.unit-test.ts | 56 +++++++++++++++++++ 2 files changed, 66 insertions(+), 2 deletions(-) create mode 100644 src/lib/proof-system/cached-lagrange-basis.unit-test.ts diff --git a/src/lib/proof-system/cache.ts b/src/lib/proof-system/cache.ts index 42343c88f..6978a8e33 100644 --- a/src/lib/proof-system/cache.ts +++ b/src/lib/proof-system/cache.ts @@ -11,7 +11,13 @@ import { jsEnvironment } from '../../bindings/crypto/bindings/env.js'; export { Cache, CacheHeader }; // internal API -export { readCache, writeCache, withVersion, cacheHeaderVersion }; +export { + readCache, + writeCache, + withVersion, + cacheHeaderVersion, + LAGRANGE_BASIS_PREFIX, +}; /** * Interface for storing and retrieving values, for caching. @@ -90,6 +96,8 @@ type StepKeyHeader = { type WrapKeyHeader = { kind: Kind; programName: string; hash: string }; type PlainHeader = { kind: Kind }; +const LAGRANGE_BASIS_PREFIX = 'lagrange-basis' as const; + /** * A header that is passed to the caching layer, to support rich caching strategies. * @@ -101,7 +109,7 @@ type CacheHeader = ( | WrapKeyHeader<'wrap-pk'> | WrapKeyHeader<'wrap-vk'> | PlainHeader<'srs'> - | PlainHeader<'lagrange-basis'> + | PlainHeader ) & CommonHeader; diff --git a/src/lib/proof-system/cached-lagrange-basis.unit-test.ts b/src/lib/proof-system/cached-lagrange-basis.unit-test.ts new file mode 100644 index 000000000..3f1880190 --- /dev/null +++ b/src/lib/proof-system/cached-lagrange-basis.unit-test.ts @@ -0,0 +1,56 @@ +import { Cache, LAGRANGE_BASIS_PREFIX } from './cache.js'; +import { SelfProof, ZkProgram } from './zkprogram.js'; +import { Field } from '../provable/field.js'; +import { it, describe, after, before } from 'node:test'; +import { expect } from 'expect'; +import { promises as fs } from 'fs'; + +const __cacheDirname = './.tmpcache'; + +const exampleProgram = ZkProgram({ + name: 'example', + publicOutput: Field, + methods: { + init: { + privateInputs: [], + async method() { + return new Field(0); + }, + }, + run: { + privateInputs: [SelfProof], + async method(p: SelfProof) { + return p.publicOutput.add(new Field(1)); + }, + }, + }, +}); + +describe('Compiling a program with a cache', () => { + const cache: Cache & { lagrangeBasisReadCount?: number } = + Cache.FileSystem(__cacheDirname); + const originalRead = cache.read; + cache.lagrangeBasisReadCount = 0; + cache.read = ({ persistentId, uniqueId, dataType }) => { + if (persistentId.startsWith(LAGRANGE_BASIS_PREFIX)) { + const readCount = cache.lagrangeBasisReadCount || 0; + cache.lagrangeBasisReadCount = readCount + 1; + } + return originalRead({ persistentId, uniqueId, dataType } as any); + }; + + before(async () => { + await fs.mkdir(__cacheDirname, { recursive: true }); + }); + + after(async () => { + await fs.rm(__cacheDirname, { recursive: true }); + }); + + it('should attempt to read lagrange basis from the cache during compile', async () => { + cache.lagrangeBasisReadCount = 0; + await exampleProgram.compile({ cache }); + expect(cache.lagrangeBasisReadCount).not.toBe(0); + cache.lagrangeBasisReadCount = 0; + }); +}); From 7958bda0393c66a7910582763e0eee21fe4ade52 Mon Sep 17 00:00:00 2001 From: Test Date: Mon, 21 Oct 2024 09:49:52 -0400 Subject: [PATCH 3/3] updating changelog and adding comment to unit test --- CHANGELOG.md | 1 + src/bindings | 2 +- src/lib/proof-system/cached-lagrange-basis.unit-test.ts | 9 +++++++++ src/mina | 2 +- 4 files changed, 12 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 612d63121..02563ddac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### Fixes +- Performance regression when compiling recursive circuits is fixed https://github.com/o1-labs/o1js/pull/1874 - Decouple offchain state instances from their definitions https://github.com/o1-labs/o1js/pull/1834 ## [1.9.0](https://github.com/o1-labs/o1js/compare/450943...f15293a69) - 2024-10-15 diff --git a/src/bindings b/src/bindings index cba94257d..e0aae7f07 160000 --- a/src/bindings +++ b/src/bindings @@ -1 +1 @@ -Subproject commit cba94257dbccfbef41a63df85ed1a6cd43d3bd29 +Subproject commit e0aae7f07ac63cd153d86a074bdccab1d8bcf230 diff --git a/src/lib/proof-system/cached-lagrange-basis.unit-test.ts b/src/lib/proof-system/cached-lagrange-basis.unit-test.ts index 3f1880190..ef6cbd751 100644 --- a/src/lib/proof-system/cached-lagrange-basis.unit-test.ts +++ b/src/lib/proof-system/cached-lagrange-basis.unit-test.ts @@ -47,6 +47,15 @@ describe('Compiling a program with a cache', () => { await fs.rm(__cacheDirname, { recursive: true }); }); + /** + * This test is a regression test for https://github.com/o1-labs/o1js/issues/1869 + * It ensures that the lagrange basis cache is accessed properly. If the file system cache is not + * read during compile, that means that the lagrange basis was returned from WASM on the first attempt. + * + * This is not necessarily a problem. If the WASM code is updated such that we expect the LB to be + * returned on the first try, and we explicitly skip the file system cache, then this test can be + * safely removed. Otherwise, a failure here probably indicates a performance regression. + */ it('should attempt to read lagrange basis from the cache during compile', async () => { cache.lagrangeBasisReadCount = 0; await exampleProgram.compile({ cache }); diff --git a/src/mina b/src/mina index 63725a490..6899054b7 160000 --- a/src/mina +++ b/src/mina @@ -1 +1 @@ -Subproject commit 63725a4909a8053371f95961f7b55c61cb13d763 +Subproject commit 6899054b745c1323b9d5bcaa62c00bed2ad1ead3