Improve performance of CSAF ingestion #458
Conversation
|
Also see: #456 |
| impl MigrationTrait for Migration { | ||
| async fn up(&self, manager: &SchemaManager) -> Result<(), DbErr> { | ||
| manager | ||
| .create_index( |
| @@ -31,7 +31,7 @@ pub enum Version { | |||
| } | |||
|
|
|||
| impl VersionInfo { | |||
| fn into_active_model(self) -> version_range::ActiveModel { | |||
| pub fn into_active_model(self) -> version_range::ActiveModel { | |||
There was a problem hiding this comment.
bonus points: if you can understand why implementing IntoActiveModel caused rustc to complain about duplicate impls, that'd be lovely. There was a trait I could use, but I couldn't make rustc happy, hence this one-off fn.
There was a problem hiding this comment.
That one was a bit weird, it seems to be coming from the blanket implementation of that trait in seaorm:
impl<A> IntoActiveModel<A> for A
where
A: ActiveModelTrait,
{
fn into_active_model(self) -> A {
self
}
}Which basically means that you can't implement IntoActiveModel for anything that should become an ActiveModel. Which feels like an API bug to me.
It also doesn't seem possible to implement From or Into for that case due to similar conflicts.
There was a problem hiding this comment.
Ok, that's not correct. Because in this case A would be VersionInfo, which doesn't implement ActiveModelTrait. And the compiler complains about impl sea_orm::IntoActiveModel<trustify_entity::version_range::ActiveModel> for <trustify_entity::version_range::Entity as sea_orm::EntityTrait>::Model being the conflict.
There was a problem hiding this comment.
Ok, I give up on the bonus points and leave it a mystery.
| package, | ||
| status, | ||
| info: VersionInfo { | ||
| scheme: "generic".to_string(), |
There was a problem hiding this comment.
not your problem, just noting: there's a few schemes which happen to match the purl type so we could avoid generic perhaps, if we know it's a python, or maven, or rust (semver) type.
| for ps in &self.entries { | ||
| // ensure a correct status, and get id | ||
| if let Entry::Vacant(entry) = checked.entry(ps.status) { | ||
| entry.insert(Self::check_status(ps.status, connection).await?); |
There was a problem hiding this comment.
Does this mean status rows can be implicitly added upon first sighting? I think that's fine.
There was a problem hiding this comment.
Nope, that just inserts it into the hash map. The idea is to look it up only once, cache the status.id and re-use it a bit further down the function. Versus: looking it up for each entry.
With the performance test, this improves the import time from ~470ms to ~240ms