Add cache hit percentage to stats #2211

Saruniks · 2024-06-21T13:14:18Z

Inspiration from #614

Feedback is welcome.

No compile requests:

Compile requests                      0
Compile requests executed             0
Cache hits                            0
Cache misses                          0
Cache hits rate                       -
Cache timeouts                        0
Cache read errors                     0
Forced recaches                       0
Cache write errors                    0
Compilation failures                  0
Cache errors                          0
Non-cacheable compilations            0
Non-cacheable calls                   0
Non-compilation calls                 0
Unsupported compiler calls            0
Average cache write               0.000 s
Average compiler                  0.000 s
Average cache read hit            0.000 s
Failed distributed compilations       0
Cache location                  Local disk: "/home/ubuntu-user/.cache/sccache"
Use direct/preprocessor mode?   yes
Version (client)                0.8.1
Max cache size                       10 GiB

Zero cache hits:

Compile requests                    397
Compile requests executed           319
Cache hits                            0
Cache misses                        316
Cache misses (C/C++)                 52
Cache misses (Rust)                 264
Cache hits rate                    0.00 %
Cache hits rate (C/C++)            0.00 %
Cache hits rate (Rust)             0.00 %
Cache timeouts                        0
Cache read errors                     0
Forced recaches                       0
Cache write errors                    0
Compilation failures                  3
Cache errors                          0
Non-cacheable compilations            0
Non-cacheable calls                  73
Non-compilation calls                 5
Unsupported compiler calls            0
Average cache write               0.004 s
Average compiler                  0.370 s
Average cache read hit            0.000 s
Failed distributed compilations       0

Non-cacheable reasons:
crate-type                           50
unknown source language              18
-                                     3
-E                                    1
incremental                           1

Cache location                  Local disk: "/home/ubuntu-user/.cache/sccache"
Use direct/preprocessor mode?   yes
Version (client)                0.8.1
Cache size                          234 MiB
Max cache size                       10 GiB

Some cache hits:

Compile requests                   1189
Compile requests executed           957
Cache hits                          632
Cache hits (C/C++)                  104
Cache hits (Rust)                   528
Cache misses                        316
Cache misses (C/C++)                 52
Cache misses (Rust)                 264
Cache hits rate                   66.67 %
Cache hits rate (C/C++)           66.67 %
Cache hits rate (Rust)            66.67 %
Cache timeouts                        0
Cache read errors                     0
Forced recaches                       0
Cache write errors                    0
Compilation failures                  9
Cache errors                          0
Non-cacheable compilations            0
Non-cacheable calls                 217
Non-compilation calls                15
Unsupported compiler calls            0
Average cache write               0.004 s
Average compiler                  0.370 s
Average cache read hit            0.004 s
Failed distributed compilations       0

Non-cacheable reasons:
crate-type                          150
unknown source language              54
-                                     7
incremental                           3
-E                                    3

Cache location                  Local disk: "/home/ubuntu-user/.cache/sccache"
Use direct/preprocessor mode?   yes
Version (client)                0.8.1
Cache size                          234 MiB
Max cache size                       10 GiB

sccache --show-adv-stats:

Compile requests                     401
Compile requests executed            319
Cache hits                           316
Cache hits (c [gcc])                  52
Cache hits (rust)                    264
Cache misses                           0
Cache hits rate                   100.00 %
Cache hits rate (c [gcc])         100.00 %
Cache hits rate (rust)            100.00 %
Cache timeouts                         0
Cache read errors                      0
Forced recaches                        0
Cache write errors                     0
Compilation failures                   3
Cache errors                           0
Non-cacheable compilations             0
Non-cacheable calls                   77
Non-compilation calls                  5
Unsupported compiler calls             0
Average cache write                0.000 s
Average compiler                   0.000 s
Average cache read hit             0.002 s
Failed distributed compilations        0

Non-cacheable reasons:
crate-type                            51
unknown source language               18
-                                      5
incremental                            2
-E                                     1

Cache location                  Local disk: "/home/ubuntu-user/.cache/sccache"
Use direct/preprocessor mode?   yes
Version (client)                0.8.1
Cache size                           235 MiB
Max cache size                        10 GiB

AJIOB · 2024-06-27T14:39:14Z

Hi @Xuanwo,

Is it OK to you?

sylvestre · 2024-06-27T14:43:52Z

src/server.rs

@@ -1510,6 +1510,17 @@ impl Default for ServerStats {
    }
 }

+macro_rules! set_percentage_stat {


why a macro and not a function?

Picked up code from #614 and experienced tunnel vision.

Function is probably a better choice here indeed.

sylvestre · 2024-06-28T10:38:01Z

Could you please add a test to verify the field is present? Thanks

codecov-commenter · 2024-06-28T10:43:32Z

Codecov Report

Attention: Patch coverage is 65.78947% with 91 lines in your changes missing coverage. Please review.

Project coverage is 40.88%. Comparing base (0cc0c62) to head (dd35cfa).
Report is 64 commits behind head on main.

Files	Patch %	Lines
src/server.rs	45.31%	18 Missing and 52 partials ⚠️
tests/helpers/mod.rs	78.26%	11 Missing and 4 partials ⚠️
tests/cache_hit_rate.rs	91.30%	6 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2211      +/-   ##
==========================================
+ Coverage   30.91%   40.88%   +9.97%     
==========================================
  Files          53       55       +2     
  Lines       20112    20394     +282     
  Branches     9755     9609     -146     
==========================================
+ Hits         6217     8339    +2122     
- Misses       7922     8004      +82     
+ Partials     5973     4051    -1922

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Saruniks · 2024-06-29T12:17:03Z

src/server.rs

+
+        let output = writer.get_output();
+
+        assert!(output.contains("Cache hits rate                        -"));


Interesting case where PerLanguageCount::all() uses counts.values().sum() and not self.adv_counts.values().sum() in case of printing advanced stats.

In practice should never happen because PerLanguageCount::increment() is used to set the counts.

Saruniks · 2024-06-29T12:24:37Z

src/server.rs

@@ -1700,7 +1780,7 @@ impl ServerInfo {

    /// Print info to stdout in a human-readable format.
    pub fn print(&self, advanced: bool) {
-        let (name_width, stat_width) = self.stats.print(advanced);
+        let (name_width, stat_width) = self.stats.print(&mut StdoutServerStatsWriter, advanced);


Intrusive way to test the code. If anyone has an idea on how to test it in a better way it would be helpful to know.

I was thinking about writing similar test like in sccache_cargo.rs (using Command::new() and capturing the output), but I was hesitant because the test would not be fast.

I was thinking about refactoring this code to free functions and testing the free functions but then it would potentially not cover the whole use case.

AJIOB · 2024-07-15T09:23:25Z

Hi @Saruniks

Is it ready to be merged?

Saruniks · 2024-07-15T09:51:17Z

Hello @AJIOB

Ready from my side. Unless perhaps someone has comments on the code and tests.

AJIOB · 2024-07-15T09:58:26Z

Hi @sylvestre & @Xuanwo
Can we merge it?

sylvestre · 2024-07-15T10:04:36Z

i was hoping to have also integration tests like in https://github.com/mozilla/sccache/blob/main/.github/workflows/integration-tests.yml
and a verification that the json files contain the fields too

sylvestre · 2024-07-15T10:06:01Z

and sorry for the latency

Saruniks · 2024-07-15T10:50:05Z

Ok. I'll add the integration test

sylvestre · 2024-07-15T10:56:52Z

many thanks :)

Saruniks · 2024-07-17T13:50:08Z

Hello,

a) I added integration tests. Not to the GitHub Actions workflow like mentioned above, but "Rust" ones (see tests/cache_hit_rate.rs).

I could add extra jobs to the GitHub Actions workflow, no problems. But I thought that what I developed could be sufficient as no integration with external tool needs to be tested. Let me know if you think otherwise.

b) Regarding the json stats:
Apologies for not mentioning it before, but I didn't develop cache hit rate display for json stats.

The problem is that json stats is just serialized structs. When "StatsFormat::Text" stats are achieved by using "complex" print method.

    match fmt {
        StatsFormat::Text => stats.print(advanced),
        StatsFormat::Json => serde_json::to_writer(&mut io::stdout(), &stats)?,
    }

I am still not sure what would be the best way to implement cache hit rate for json stats. Maybe custom serialize implementation would work (and not derive trait like currently), but then it could be considered magic-like. But then calculating cache hit rate on every stats change could be considered too wasteful (or considered as calculated value anti-pattern).

So not sure how to proceed with json stats.

Saruniks · 2024-07-17T13:50:59Z

tests/helpers/mod.rs

No new code in this file, only moved things around for deduplication

sylvestre · 2024-07-17T14:25:45Z

don't bother for the json output
thanks!

sylvestre · 2024-07-17T14:26:06Z

I guess you saw that some tests are failing

Saruniks · 2024-07-17T15:15:14Z

I fixed the issue with failing tests. Checks should pass now.

sylvestre reviewed Jun 27, 2024

View reviewed changes

Saruniks commented Jun 29, 2024

View reviewed changes

Saruniks added 4 commits July 17, 2024 16:44

Add cache hit percentage to stats

8f15229

Change macro to a function

665075a

Add tests

f5b64f1

Add integration tests

dd35cfa

Saruniks force-pushed the add-hit-miss-ratio-to-stats branch from c50cc12 to dd35cfa Compare July 17, 2024 13:45

Saruniks commented Jul 17, 2024

View reviewed changes

tests/helpers/mod.rs Outdated

Copy link

Author

Saruniks Jul 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No new code in this file, only moved things around for deduplication

Fix undeclared types

1234cef

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add cache hit percentage to stats #2211

Add cache hit percentage to stats #2211

Saruniks commented Jun 21, 2024 •

edited

Loading

AJIOB commented Jun 27, 2024

sylvestre Jun 27, 2024

Saruniks Jun 28, 2024

sylvestre commented Jun 28, 2024

codecov-commenter commented Jun 28, 2024 •

edited

Loading

Saruniks Jun 29, 2024

Saruniks Jun 29, 2024

AJIOB commented Jul 15, 2024

Saruniks commented Jul 15, 2024

AJIOB commented Jul 15, 2024

sylvestre commented Jul 15, 2024 •

edited

Loading

sylvestre commented Jul 15, 2024

Saruniks commented Jul 15, 2024

sylvestre commented Jul 15, 2024

Saruniks commented Jul 17, 2024

Saruniks Jul 17, 2024

sylvestre commented Jul 17, 2024

sylvestre commented Jul 17, 2024

Saruniks commented Jul 17, 2024


		let output = writer.get_output();

		assert!(output.contains("Cache hits rate -"));

Add cache hit percentage to stats #2211

Are you sure you want to change the base?

Add cache hit percentage to stats #2211

Conversation

Saruniks commented Jun 21, 2024 • edited Loading

AJIOB commented Jun 27, 2024

sylvestre Jun 27, 2024

Choose a reason for hiding this comment

Saruniks Jun 28, 2024

Choose a reason for hiding this comment

sylvestre commented Jun 28, 2024

codecov-commenter commented Jun 28, 2024 • edited Loading

Codecov Report

Saruniks Jun 29, 2024

Choose a reason for hiding this comment

Saruniks Jun 29, 2024

Choose a reason for hiding this comment

AJIOB commented Jul 15, 2024

Saruniks commented Jul 15, 2024

AJIOB commented Jul 15, 2024

sylvestre commented Jul 15, 2024 • edited Loading

sylvestre commented Jul 15, 2024

Saruniks commented Jul 15, 2024

sylvestre commented Jul 15, 2024

Saruniks commented Jul 17, 2024

Saruniks Jul 17, 2024

Choose a reason for hiding this comment

sylvestre commented Jul 17, 2024

sylvestre commented Jul 17, 2024

Saruniks commented Jul 17, 2024

Saruniks commented Jun 21, 2024 •

edited

Loading

codecov-commenter commented Jun 28, 2024 •

edited

Loading

sylvestre commented Jul 15, 2024 •

edited

Loading