Repository navigation
JJ adapters - #41
JJ adapters#41sidracha wants to merge 1 commit into
Conversation
9d160ba to
7472d4c
Compare
| } | ||
|
|
||
| async fn read_operation(&self, id: &OperationId) -> OpStoreResult<Operation> { | ||
| if id.as_bytes() == cc_common::ROOT_OPERATION_ID_BYTES { |
There was a problem hiding this comment.
Ideally the root operation is persisted in the database when the repo is created and you would not need special logic for it here? But maybe you need it to bootstrap things?
There was a problem hiding this comment.
Its required here, yeah. jj-lib does this same thing
see
| Ok(ViewId::from_bytes(&view_id_bytes)) | ||
| } | ||
|
|
||
| async fn resolve_operation_id_prefix( |
There was a problem hiding this comment.
I suppose this function is never going to be invoked on the server and that's why you can get away with this implementation, rigth?
There was a problem hiding this comment.
Yes, its never going to be invoked on the server. But its the same thing on the client side. its currently a TODO there as well, to support this Operation id prefix matching. currently only full op id matching is supported
| &self.empty_tree_id | ||
| } | ||
|
|
||
| fn concurrency(&self) -> usize { |
There was a problem hiding this comment.
I see the lifetime of ServerBackend is just within a reconcile op heads request, so I suppose this setting only ever affects reconcile op heads RPC performance. RepoLoader::resolve_op_heads calls RepoLoader::merge_operations which calls rebase_descendants: https://github.com/jj-vcs/jj/blob/e9f48b3e4c1c5e816fbbfbfd06c154a6a96b0215/lib/src/repo.rs#L878
I am not sure, but maybe rebase_descendants ends up in code that uses this concurrency setting, perhaps something like https://github.com/jj-vcs/jj/blob/e9f48b3e4c1c5e816fbbfbfd06c154a6a96b0215/lib/src/tree_merge.rs#L237 ... I don't know for sure. In summary: maybe you should set this some tunable value, or leave a TODO to explore this further.
There was a problem hiding this comment.
Yeah, this only affects reconcile performance. the ServerBackend and database store methods are all async, so many database requests can be sent out concurrently and work...
I am not sure what to set this parameter to. Google sets it to 100
set it to 100 for now, added a comment explaining why.
| } | ||
|
|
||
| #[async_trait] | ||
| impl OpHeadsStore for ServerOpHeadsStore { |
There was a problem hiding this comment.
Not actionable: it is unfortunate that you have to write so much logic to just get the RepoLoader::resolve_op_heads algorithm. In the long run when the commit cloud is polished and productionized I think this is likely to change. Maybe the commit cloud server should bite the bullet and do as we do in Google's jj server and use the full-blown jj lib? I don't have a concrete picture of that would be like and is out of scope of your project though.
7472d4c to
3a5cf8a
Compare
Implement the traits for ServerBackend, ServerOpStore, ServerOpHeadsStore, ServerIndexStore, and ServerSubModuleStore. jj-lib repo_loader.load_at_head (which calls jj-lib resolve_op_heads function) is called for the main reconciliation engine. RepoLoader takes in jj Structs (settings, store, op_store, op_heads_store, index_store, submodule_store) as arguments to the constructor. Server-side implementations of these, which directly call Store trait functions (bypassing any RPCs) are required to pass them into a server-side construction of the RepoLoader. The ServerIndexLoader is implemented in-memory to increase performance, while the default jj-lib IndexLoader creates files on disk, a very expensive operation.
3a5cf8a to
7714ee8
Compare
server: implement jj-lib trait adapters for the server
Implement the traits for ServerBackend, ServerOpStore, ServerOpHeadsStore, ServerIndexStore, and ServerSubModuleStore. jj-lib repo_loader.load_at_head (which calls jj-lib resolve_op_heads function) is called for the main reconciliation engine. RepoLoader takes in jj Structs (settings, store, op_store, op_heads_store, index_store, submodule_store) as arguments to the constructor. Server-side implementations of these, which directly call Store trait functions (bypassing any RPCs) are required to pass them into a server-side construction of the RepoLoader.
The ServerIndexLoader is implemented in-memory to increase performance, while the default jj-lib IndexLoader creates files on disk, a very expensive operation.