Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ICU-22991 Reduce fStamp to 8 bits #3323

Merged
merged 1 commit into from
Jan 7, 2025

Conversation

FrankYFTang
Copy link
Contributor

@FrankYFTang FrankYFTang commented Dec 31, 2024

Reduce 24x3=72 bytes per Calendar object by changing from 32 bits integer to 8 bits integer for the stamp array.
Reset the fNextStamp whenever we clear the fStamp. There are at most 24 different stamp values stored inside fStamp so using 7 bits to store up to 127 different one should be enough.

Also change to use bit fields for:
fIsTimeSet, fAreFieldsSet, fAreAllFieldsSet, fAreFieldsVirtuallySet, fLenient, fRepeatedWallTime, fSkippedWallTime, fFirstDayOfWeek, fWeekendOnset, fWeekendCease to save more space

Checklist

  • Required: Issue filed: ICU-22991
  • Required: The PR title must be prefixed with a JIRA Issue number. Example: "ICU-1234 Fix xyz"
  • Required: Each commit message must be prefixed with a JIRA Issue number. Example: "ICU-1234 Fix xyz"
  • Issue accepted (done by Technical Committee after discussion)
  • Tests included, if applicable
  • API docs and/or User Guide docs changed or added, if applicable

@FrankYFTang FrankYFTang force-pushed the ICU-22991-reduceStamp branch from 2673453 to e087df3 Compare December 31, 2024 05:16
@jira-pull-request-webhook
Copy link

Notice: the branch changed across the force-push!

  • icu4c/source/i18n/calendar.cpp is different
  • icu4j/main/core/src/main/java/com/ibm/icu/util/Calendar.java is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

@FrankYFTang FrankYFTang force-pushed the ICU-22991-reduceStamp branch from e087df3 to 98b06dd Compare December 31, 2024 05:41
@jira-pull-request-webhook
Copy link

Notice: the branch changed across the force-push!

  • icu4c/source/i18n/calendar.cpp is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

@FrankYFTang FrankYFTang force-pushed the ICU-22991-reduceStamp branch from 98b06dd to b31ce36 Compare December 31, 2024 05:45
@jira-pull-request-webhook
Copy link

Notice: the branch changed across the force-push!

  • icu4j/main/core/src/main/java/com/ibm/icu/util/Calendar.java is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

icu4c/source/i18n/unicode/calendar.h Outdated Show resolved Hide resolved
icu4c/source/i18n/unicode/calendar.h Outdated Show resolved Hide resolved
@FrankYFTang FrankYFTang force-pushed the ICU-22991-reduceStamp branch from 01ed02b to 4cc9027 Compare January 4, 2025 00:51
@jira-pull-request-webhook
Copy link

Notice: the branch changed across the force-push!

  • icu4c/source/i18n/calendar.cpp is different
  • icu4c/source/i18n/unicode/calendar.h is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

@FrankYFTang FrankYFTang requested a review from markusicu January 4, 2025 00:56
markusicu
markusicu previously approved these changes Jan 6, 2025
@FrankYFTang FrankYFTang force-pushed the ICU-22991-reduceStamp branch from c49a430 to cba1319 Compare January 6, 2025 22:07
@jira-pull-request-webhook
Copy link

Hooray! The files in the branch are the same across the force-push. 😃

~ Your Friendly Jira-GitHub PR Checker Bot

@FrankYFTang
Copy link
Contributor Author

Somehow for MSVC, we need one more bit to store the enum. May be a sign conversion bug.

@FrankYFTang FrankYFTang requested a review from markusicu January 6, 2025 22:09
@FrankYFTang FrankYFTang merged commit 6091406 into unicode-org:main Jan 7, 2025
101 checks passed
@FrankYFTang FrankYFTang deleted the ICU-22991-reduceStamp branch January 7, 2025 02:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants