perf: Preallocate records #1185
Conversation
This comment has been minimized.
This comment has been minimized.
zlangley
force-pushed
the
perf/preallocate
branch
from
54eaefc to
164506a
Compare
This comment has been minimized.
This comment has been minimized.
zlangley
force-pushed
the
perf/preallocate
branch
from
164506a to
dc32f47
Compare
This comment has been minimized.
This comment has been minimized.
zlangley
force-pushed
the
perf/preallocate
branch
from
dc32f47 to
e7d88f4
Compare
This comment has been minimized.
This comment has been minimized.
| } | ||
|
|
||
| impl Default for MemoryConfig { | ||
| fn default() -> Self { | ||
| Self::new(29, 1, 29, 29, 17, 64) | ||
| Self::new(29, 1, 29, 29, 17, 64, 1 << 24) |
There was a problem hiding this comment.
224 seems pretty high, maybe just 220?
There was a problem hiding this comment.
This is for memory. If a chip has up to 2^20 records and we have dozens of chips and each record does several accesses, then 2^24 might still be small?
There was a problem hiding this comment.
can you lmk the execution performance diff (via criterion bench) between 2^20 and 2^24, just for a sense?
There was a problem hiding this comment.
from 1 << 20 to 1 << 24 (regex_execute bench):
regex/execute time: [377.90 ms 391.66 ms 405.92 ms]
change: [-11.309% -6.3323% -0.8434%] (p = 0.05 < 0.05)
Change within noise threshold.
on the border of statistical signficance
| @@ -201,7 +201,7 @@ where | |||
| Self { | |||
| adapter, | |||
| core, | |||
| records: vec![], | |||
| records: Vec::with_capacity(1 << 20), | |||
There was a problem hiding this comment.
this should use access_capacity
There was a problem hiding this comment.
Do you have any suggestions for how to access config from here?
(Also I think this is a different parameter than access_capacity; access_capacity is the number of memory record IDs, which should be several factors larger than any one chip's records.len(), both because every chip contributes to memory records and because each chip record corresponds to several memory records.)
There was a problem hiding this comment.
I guess we could add it as a field in OfflineMemory...
There was a problem hiding this comment.
It seems weird because this is more about the number of instructions executed by any one chip rather than anything memory-related.
There was a problem hiding this comment.
ok we can address later then
There was a problem hiding this comment.
at least make it a private constant
Commit: 6471801 |
No description provided.