Skip to content

Added racing DMA LZ4 & LZ4T implementations#824

Merged
aglab2 merged 8 commits intoHackerN64:develop/2.4.0from
aglab2:lz4t
Dec 29, 2024
Merged

Added racing DMA LZ4 & LZ4T implementations#824
aglab2 merged 8 commits intoHackerN64:develop/2.4.0from
aglab2:lz4t

Conversation

@aglab2
Copy link
Collaborator

@aglab2 aglab2 commented Aug 18, 2024

No description provided.

@aglab2 aglab2 requested a review from gheskett as a code owner August 18, 2024 12:09
@aglab2 aglab2 changed the base branch from master to develop/2.4.0 August 18, 2024 12:16

#ifdef PUPPYPRINT_DEBUG
if (gLoadLevel && gLoadLevelAreaTime) {
u32 totalTime = gLoadLevelAreaTime + (osGetCount() - first);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Isn't gLoadLevelAreaTime unnecessary? I feel like it's getting counted more than once.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I did not write this code, @FazanaJ needs to elaborate. From what I see it is needed though.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is necessary

Copy link
Collaborator

Choose a reason for hiding this comment

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

No, looking back at it this is definitely wrong. gLoadLevelAreaTime can be used as a factor to the if statement I guess, but just the osGetCount() - first is all that's needed here. init_level is getting invoked after first has already been set.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ultimately this was the product of a 5 minute, if that, attempt to get a better readout. I never checked that it was accurate, just representative. Ideally something better would be done to give an accurate readout but that was effort I wasn't gonna do at the time

Copy link
Collaborator

@gheskett gheskett Sep 1, 2024

Choose a reason for hiding this comment

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

Actually on second glance, this is still extremely inaccurate because it also throws an entire update_level call into the reading.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Either the profiling on this should be relatively accurate or this needs to be purged from the repo entirely, since we're better off with no profiling than bad profiling.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have addressed this issue. I am measuring time between level_cmd_init_level (called right before loading banks) and init_level (called after all area loads commands).

@gheskett gheskett added enhancement New feature or request performance Issue or feature related to game performance labels Aug 28, 2024
@gheskett gheskett added this to the 2.4 milestone Aug 28, 2024
gLoadLevelAreaTime = osGetCount() - first;
if (gInitLevelTime) {
u32 totalTime = osGetCount() - gInitLevelTime;
append_puppyprint_log("Level loaded in %2.3fs.", (f64) (f32)((totalTime) / 46875000.0f));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please use something like OS_CYCLES_TO_USEC rather than this magic constant.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Constant was removed and replaced with OS_CYCLES_TO_USEC

Copy link
Collaborator

@gheskett gheskett left a comment

Choose a reason for hiding this comment

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

Looks good to me now.

@gheskett gheskett requested a review from arthurtilly September 5, 2024 05:15
@aglab2 aglab2 merged commit d43157c into HackerN64:develop/2.4.0 Dec 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request performance Issue or feature related to game performance

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants