Repository navigation
Stop the endless page walk and build the state schema faster - #474
Draft
alexluckett wants to merge 3 commits into
Draft
alexluckett wants to merge 3 commits into
alexluckett wants to merge 3 commits into
Conversation
getFormContext walks the pages from the start page until it gets to the requested page. After the last page the next path is undefined, and findPage read that as the root path '/'. A form with a page at '/' then continued the walk from that page with no end, for each request to a page that was off the route. The walk is synchronous, so the request blocked the process. findPage now finds no page when it gets no path, so the walk ends after the last page. The walk also remembers the pages it has visited and throws a PageRouteCycleError when the route returns to one of them. This covers page paths that are equal after normalisation, such as '/name' and '/name/', and V1 next links that point back to an earlier page.
makeFilteredSchema joined the state schema of each relevant page onto one growing schema. Joi copies both schemas on each concat, so the work grew with the square of the page count. For a form with 239 relevant pages the build took about 650 ms of synchronous work, on each request late in the journey and two times on the summary page. The schemas are now joined in pairs, then the results in pairs, until one is left. Each key is copied about log2(pages) times. The same form builds in about 28 ms. It still uses concat, so object-level rules such as the country rule of EastingNorthingField stay in the joined schema, and the keys keep the page order.
The pairs loop made the build fast but was hard to read. makeFilteredSchema now joins the page schemas in batches of 32, then joins the batch schemas. Both steps use the same plain helper, joinSchemas, which adds one schema at a time. Inside a batch the growing schema has 32 pages at most, and only the few joins between batches work on a large schema. For a form with 239 relevant pages the build takes about 45 to 65 ms, against about 650 to 870 ms for one join for each page on the full schema. The pages are still joined first to last, so the keys, the object-level rules and the order of errors are the same. A new test covers a form with more pages than one batch.
|
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Proposed change
This fixes two problems in
getFormContext, which could block or slow the main node thread.Problem 1: The page walk may never end.
/made the walk loop with no end. This occurred when a user requested a page that was off the route.findPagenow finds no page when it gets no path. Thus the walk stops after the last page.PageRouteCycleErrorwhen the route comes back to a page. This also stops loops from paths such as/nameand/name/, and from V1nextlinks that point back.Problem 2: The state schema was slow to build.
makeFilteredSchemaadded one page at a time to one large schema. Joi copies the schema on each step, so large forms were slow.Performance
Measured on a form with 243 pages, for a request to the summary page. The number of relevant pages changes with the answers. Each time is the middle value of 9 runs on one laptop.
getFormContext, beforegetFormContext, afterThe summary page builds the schema two times: one time in
getFormContextand one time inSummaryViewModel.A request for a page that is off the route, on the same form with its page at
/: before, the request had no end. After,getFormContexttakes 16 ms.Jira ticket:
Type of change
Checklist
README.mdanddocs/*(where appropriate, e.g. new features).npm run test).npm run lint).npm run format).