Build times have deteriorated significantly #3610
Comments
|
That looks bad. Did you also try turning the |
|
screeenshot of timings on current main: |
|
Okay the timings above are all crap, turns out sccache wasnt actually disabled because I have it configured via
Good point, I didnt remember that change. I tried this on 0.18.0 (for faster builds):
So this is a huge difference and the dev profile should definitely be added back. Debug symbols are very rarely needed, and if so the lines can be temporarily commented out. |
This significantly speeds up builds: - with strip symbols and debug 0: 43s - without: 169s
|
Doesn't look extremely useful, but: and: |
|
Hey just out of curiosity are you guys building your dev environments directly from your device or is there some standardization? |
|
I measured the impact of #3420 with a more consistent measurement method. I made one folder contain the contents of the repository before the merge, and another folder with the contents after the merge: I created this script called I ran Running one compilation at a time could cause the 2 results to be affected by different conditions (such as different CPU usage of other processes). For both compilations, the timing was extremely similar for all dependencies and some lemmy crates. When switching the editor view between out1.txt and out2.txt, I wasn't totally sure that they were different processes until I saw the inconsistent ordering. The build time of |
|
@dullbananas You are testing full builds while Im testing incremental builds so its a completely different thing. Also the env vars you are setting might affect the results. And of course running two builds in parallel will slow both of them down too. So you should check with the steps described above. One problem Im having is that the timings are extremely inconsistent, one run is over 5 minutes and the next identical build is only 2 minutes. Almost like theres still some caching active. @phiresky What does that mean? @JustAnotherSoftwareDeveloper I dont understand, this requires only |
Nothing changeable for this issue I guess, you can ignore it. I also had the thing where incremental builds sometimes took 2min and sometimes 4min, even though there was no real difference. Also sometimes it started rebuilding from scratch when changing these settings and sometimes it didn't. |
|
I ran it again without db_views: 25.1 to 390.5 db_views_actor: 20.5 to 196.8 |
|
I changed After compiling twice in parallel like before, the profiling data for a single crate can be compared by running this: Here's the top part of the diff for |
|
Maybe this is because of the increased amount of lifetimes caused by replacing |
|
This is a difficult one for sure. Spent a few hours trying some tricks and nothing made a dent. My guess is some weird interaction with Diesel types. |
|
It's not perfect, but I came up with a solution that compiles and I think should work, with the caveat that the pub type ActualDbPool = Pool<AsyncPgConnection>;
pub struct DbPool {
pub pool: ActualDbPool,
conn: Option<DbPoolConn>
}
pub enum DbPoolConn {
Orig(PooledConnection<AsyncPgConnection>),
Ref(*mut AsyncPgConnection)
}
unsafe impl Send for DbPoolConn {}
unsafe impl Sync for DbPoolConn {}
impl Deref for DbPoolConn {
type Target = AsyncPgConnection;
fn deref(&self) -> &Self::Target {
match self {
Self::Orig(conn) => conn,
Self::Ref(conn) => unsafe { &**conn }
}
}
}
impl DerefMut for DbPoolConn {
fn deref_mut(&mut self) -> &mut Self::Target {
match self {
Self::Orig(conn) => conn,
Self::Ref(conn) => unsafe { &mut **conn }
}
}
}
impl DbPool {
pub fn new(pool: ActualDbPool) -> Self {
Self {
pool,
conn: None
}
}
pub async fn connect(&mut self) -> Result<DbPoolConn, DieselError> {
if self.conn.is_none() {
self.conn = Some(DbPoolConn::Orig(self.pool.get().await.map_err(|e| QueryBuilderError(e.into()))?));
}
Ok(self.conn.take().unwrap())
}
pub fn is_connected(&self) -> bool {
self.conn.is_some()
}
pub fn from_connection(pool: &DbPool, conn: DbPoolConn) -> Self {
Self {
pool: pool.pool.clone(),
conn: Some(conn)
}
}
/// # Safety
/// You MUST treat `conn` as fully mutably borrowed (unusable) for the lifetime of the returned `DbPool`.
pub unsafe fn from_raw_connection(pool: &DbPool, conn: &mut AsyncPgConnection) -> Self {
Self {
pool: pool.pool.clone(),
conn: Some(DbPoolConn::Ref(conn as *mut AsyncPgConnection))
}
}
}
#[macro_export]
macro_rules! try_join_with_pool {
($pool:ident => ($($func:expr),+)) => {{
// Check type
let _: &mut $crate::utils::DbPool = $pool;
if $pool.is_connected() {
// Run sequentially
async {
Ok(($({
// `?` prevents the error type from being inferred in an `async` block, so `match` is used instead
match ($func)($pool).await {
::core::result::Result::Ok(__v) => __v,
::core::result::Result::Err(__v) => return ::core::result::Result::Err(__v),
}
}),+))
}.await
} else {
// Run concurrently with `try_join`
::futures::try_join!(
$(async {
let mut tmp = $crate::utils::DbPool::new($pool.pool.clone());
($func)(&mut tmp).await
}),+
)
}
}};
}It mostly works like the old version. And the unsafe+raw pointer are only necessary for one thing as far as I can tell, which is when you need to use a Also I don't have an environment set up yet so I haven't tested it - there may be bugs. |
|
Unsafe code should be a last resort. I'm currently trying to reduce compile time by removing use of Also, I recently realized that |
|
@dullbananas Maybe instead of those structs it would be easier to make the trait GetConn {
fn get_conn(&mut self) -> &mut AsyncPgConnection;
}
impl GetConn for Pool<AsyncPgConnection> { ...self.get() }
impl GetConn for Object<AsyncPgConnection> { &mut self }and then just pass around |
|
#3637 is a major improvement, it improves build time from 2m 16s to 42s in my measurements. This completely resolves the issue in my view, although it can never hurt to improve compile time further. Using unsafe is not an option though because we have zero experience with that, and its too risky in a web server which is exposed to the internet. |
|
I think this is making it slow: diesel-rs/diesel#3223 |
…#3610) (LemmyNet#3611) * Add dev profile to strip symbols and disable debug info (ref LemmyNet#3610) This significantly speeds up builds: - with strip symbols and debug 0: 43s - without: 169s * add comment, no strip symbols
|
It looks like removing TypedBuilder completely fixed the promblem caused by #3420. This is supported by the lack of significant change in build time of db_views_moderator, which didn't use TypedBuilder. So changing DbPool is unlikely to improve build time. |
|
Gonna close this issue then. |
I noticed that incremental builds are now much slower than they used to be, so I did some testing. Im doing this with the following commands:
I only did one run per commit and the results arent exactly consistent, but we can still draw some conclusions:
So #3581 doesnt improve compile time at all.Edit: Looks like this actually helps a lot but I want to do more testing.
#3420 is the worst offender, the build time is almost than doubled. We definitely need to find a solution for this, or otherwise revert the change
0.18.1 is much slower, specifically for compiling lemmy_server according to the html report. This is most likely from adding additional code in that crate like prometheus metrics.
The solution is to move as much as possible into lemmy_routes so that it can be compiled in parallel with api and apub crates.cc @dullbananas
Edit: All measurements updated so they are not polluted by sccache. They should be quite accurate now.
The text was updated successfully, but these errors were encountered: