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

change the way to caculate the size of string #7

Closed
wants to merge 1 commit into from
Closed

change the way to caculate the size of string #7

wants to merge 1 commit into from

Conversation

j-joker
Copy link

@j-joker j-joker commented Oct 6, 2016

I think this way is better

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.5%) to 84.912% when pulling 0324aff on j-joker:master into 2384653 on lemire:master.

@lemire
Copy link
Owner

lemire commented Oct 6, 2016

Please merge the following commit
a5886f7

and run mvn test before and after your change. It will measure in a rough but sufficiently accurate manner the running time of the string estimation.

We want to make sure that we do not degrade the performance since this function is called repeatedly, possibly millions of times. It also does something that is relatively unimportant (produce a memory usage estimation) so we do not ever want it to have an impact on performance.

Here is what it might look like...

$ mvn test
(...)
Running com.google.code.externalsorting.ExternalSortTest
#ignore = 67412000
[performance] String size estimator uses 1.116796875 ns per string
#ignore = 67412000
[performance] String size estimator uses 1.120703125 ns per string
#ignore = 67412000
[performance] String size estimator uses 1.1216796875 ns per string
#ignore = 67412000
[performance] String size estimator uses 1.116796875 ns per string
#ignore = 67412000
[performance] String size estimator uses 1.1138671875 ns per string
#ignore = 67412000
[performance] String size estimator uses 1.116796875 ns per string
#ignore = 67412000
[performance] String size estimator uses 1.1197265625 ns per string
#ignore = 67412000
[performance] String size estimator uses 1.116796875 ns per string
#ignore = 67412000
[performance] String size estimator uses 1.112890625 ns per string
#ignore = 67412000
[performance] String size estimator uses 1.116796875 ns per string

long offset= unsafe.objectFieldOffset(f);
maxSize=Math.max(maxSize,offset);
}
return (maxSize/8+1)*8;
Copy link
Owner

Choose a reason for hiding this comment

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

I think that the figure "8" here assumes are 64-bit JVM.

* @param s The string to estimate memory footprint.
* @return The <strong>estimated</strong> size in bytes.
*/
public static long estimatedSizeOf(String s) {
Copy link
Owner

Choose a reason for hiding this comment

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

I am very concerned about the performance of this function. We designed the estimator to be dirt cheap ((s.length() * 2) + OBJ_OVERHEAD). You are proposing something that looks much more expensive. See my comments on the PR about benchmarking this function.

As the name implies, I think we want an estimation, and we want to get it in not much more than 1 ns on a desktop PC.

@lemire
Copy link
Owner

lemire commented Oct 29, 2017

I an going to close this since the submitter never got back to us.

@lemire lemire closed this Oct 29, 2017
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.

None yet

3 participants