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.

Sign up for GitHub

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

Jump to bottom

Clean up code for FrequentObjectsStats #20445

Merged
merged 1 commit into from
Oct 31, 2024
Merged

Clean up code for FrequentObjectsStats #20445

merged 1 commit into from
Oct 31, 2024

Conversation

LinHu2016
Copy link
Contributor

@LinHu2016 LinHu2016 commented Oct 30, 2024 •
edited
Loading

replace U_32 with uint32_t
replace UDATA with uintptr_t
correct code formats

fix:#20449

@LinHu2016
Copy link
Contributor Author

@dmitripivkine please review the changes, Thanks

dmitripivkine
dmitripivkine reviewed Oct 30, 2024
View reviewed changes
Copy link
Contributor

@dmitripivkine dmitripivkine 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, a few minor missed points

runtime/gc_stats/FrequentObjectsStats.cpp Outdated
@@ -81,23 +81,23 @@ MM_FrequentObjectsStats::kill(MM_EnvironmentBase *env)
void
MM_FrequentObjectsStats::traceStats(MM_EnvironmentBase *env)
{
UDATA i;
uintptr_t i;
Copy link
Contributor

Choose a reason for hiding this comment

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

please set initial value 0

runtime/gc_stats/FrequentObjectsStats.cpp Outdated
for(i=0; i < spaceSavingGetCurSize(_spaceSaving) && i < _topKFrequent; i++){
J9Class * clazz = (J9Class *) spaceSavingGetKthMostFreq(_spaceSaving,i+1);
UDATA count = spaceSavingGetKthMostFreqCount(_spaceSaving,i+1);
for (i=0; i < spaceSavingGetCurSize(_spaceSaving) && i < _topKFrequent; i++) {
Copy link
Contributor

Choose a reason for hiding this comment

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

please add spaces around "="

runtime/gc_stats/FrequentObjectsStats.cpp Outdated
UDATA count = spaceSavingGetKthMostFreqCount(_spaceSaving,i+1);
for (i=0; i < spaceSavingGetCurSize(_spaceSaving) && i < _topKFrequent; i++) {
J9Class *clazz = (J9Class *) spaceSavingGetKthMostFreq(_spaceSaving, i+1);
uintptr_t count = spaceSavingGetKthMostFreqCount(_spaceSaving, i+1);
Copy link
Contributor

Choose a reason for hiding this comment

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

please add spaces around "+", here and above

runtime/gc_stats/FrequentObjectsStats.cpp Outdated
@@ -106,24 +106,23 @@ MM_FrequentObjectsStats::traceStats(MM_EnvironmentBase *env)

utf = J9ROMCLASS_CLASSNAME(arrayClass->leafComponentType->romClass);
Trc_MM_FrequentObjectStats_AllocationCacheIndexableObjectAllocation(
vmThread, clazz, J9UTF8_LENGTH(utf), J9UTF8_DATA(utf), arity*2, brackets, count, (UDATA) (((float)count)*sampleFreq));
}else{
vmThread, clazz, J9UTF8_LENGTH(utf), J9UTF8_DATA(utf), arity*2, brackets, count, (uintptr_t) (((float)count)*sampleFreq));
Copy link
Contributor

Choose a reason for hiding this comment

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

spaces around *

runtime/gc_stats/FrequentObjectsStats.cpp Outdated
{
UDATA i;
OMRSpaceSaving* spaceSaving = frequentObjectsStats->_spaceSaving;
uintptr_t i;
Copy link
Contributor

Choose a reason for hiding this comment

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

add initial value 0

runtime/gc_stats/FrequentObjectsStats.cpp Outdated
for(i = 0; i < spaceSavingGetCurSize(spaceSaving); i++ ){
spaceSavingUpdate(_spaceSaving, spaceSavingGetKthMostFreq(spaceSaving,i+1), spaceSavingGetKthMostFreqCount(spaceSaving,i+1));
for (i = 0; i < spaceSavingGetCurSize(spaceSaving); i++) {
spaceSavingUpdate(_spaceSaving, spaceSavingGetKthMostFreq(spaceSaving, i+1), spaceSavingGetKthMostFreqCount(spaceSaving, i+1));
Copy link
Contributor

Choose a reason for hiding this comment

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

spaces around +, twice

runtime/gc_stats/FrequentObjectsStats.cpp Outdated
UDATA arity = arrayClass->arity;
J9UTF8* utf;
uintptr_t arity = arrayClass->arity;
J9UTF8 *utf;
Copy link
Contributor

Choose a reason for hiding this comment

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

initial value NULL

amicic
amicic reviewed Oct 30, 2024
View reviewed changes
runtime/gc_stats/FrequentObjectsStats.hpp Outdated
@@ -92,15 +92,15 @@ class MM_FrequentObjectsStats : public MM_Base
* @param portLibrary the port library
* @param k the number of frequent objects we'd like to accurately report
*/
MM_FrequentObjectsStats(J9PortLibrary *portLibrary, U_32 k=TOPK_FREQUENT_DEFAULT)
MM_FrequentObjectsStats(J9PortLibrary *portLibrary, uint32_t k=TOPK_FREQUENT_DEFAULT)
Copy link
Contributor

Choose a reason for hiding this comment

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

spaces around =

runtime/gc_stats/FrequentObjectsStats.cpp Outdated
Trc_MM_FrequentObjectStats_AllocationCacheObjectAllocation(
vmThread, clazz, J9UTF8_LENGTH(J9ROMCLASS_CLASSNAME(clazz->romClass)), J9UTF8_DATA(J9ROMCLASS_CLASSNAME(clazz->romClass)), clazz->totalInstanceSize, count, (UDATA) (((float)count)*sampleFreq));
vmThread, clazz, J9UTF8_LENGTH(J9ROMCLASS_CLASSNAME(clazz->romClass)), J9UTF8_DATA(J9ROMCLASS_CLASSNAME(clazz->romClass)), clazz->totalInstanceSize, count, (uintptr_t) (((float)count)*sampleFreq));
Copy link
Contributor

Choose a reason for hiding this comment

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

spaces around operator *

runtime/gc_stats/FrequentObjectsStats.cpp Outdated
J9VMThread *vmThread = (J9VMThread *)env->getOmrVMThread()->_language_vmthread;
MM_GCExtensionsBase *extensions = env->getExtensions();
/* Should convert easily as frequentObjectAllocationSamplingRate is a UDATA representing a percentage*/
/* Should convert easily as frequentObjectAllocationSamplingRate is a uintptr_t representing a percentage*/
float sampleFreq = 100/((float) extensions->frequentObjectAllocationSamplingRate);
Copy link
Contributor

Choose a reason for hiding this comment

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

spaces around operator /

@LinHu2016
Clean up code for FrequentObjectsStatsClean up code for
13043e6 
Clean up code for FrequentObjectsStats

replace U_32 with uint32_t
replace UDATA with uintptr_t
correct code formats

Signed-off-by: lhu <[email protected]>
@dmitripivkine
Copy link
Contributor

Jenkins compile win jdk11

@dmitripivkine dmitripivkine merged commit 8d417d6 into eclipse-openj9:master Oct 31, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@amicic amicic amicic approved these changes

@dmitripivkine dmitripivkine dmitripivkine approved these changes

Assignees

@LinHu2016 LinHu2016

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@LinHu2016 @dmitripivkine @amicic