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

Used new hashed keywords, buffered file read #152

Open
wants to merge 18 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@tigrazone

tigrazone commented Dec 8, 2017

18% - 25% file read speedup,
more clear code,
keywords list

@tigrazone

This comment has been minimized.

Owner

tigrazone commented on b2f07d1 Dec 8, 2017

10% faster obj file load.
multichar keywords searches grouped in code and then strncmp() changed to compare of 2 ints.
keywords and token ids declared in one place

tigrazone added some commits Dec 8, 2017

@tigrazone

This comment has been minimized.

Owner

tigrazone commented on 5d7f6bf Dec 8, 2017

scene2.obj file size: 90m original tiny_obj_loader 4.4s buffered+hashed keywords 3.6s, my is faster 18%
hairball.obj file size: 230m original tiny_obj_loader 9.9s buffered+hashed keywords 7.7s, my is faster 22%
rungholt.obj file size: 275m original tiny_obj_loader 14.5s buffered+hashed keywords 11.9s, my is faster 18%
powerplant.obj file size: 817m original tiny_obj_loader 37.4s buffered+hashed keywords 28.1s, my is faster 24.9%
sponza_with_statues.objj file size: 50m original tiny_obj_loader 2.0s buffered+hashed keywords 1.6s, my is faster 20%

tigrazone added some commits Dec 8, 2017

tigrazone
tigrazone
@syoyo

This comment has been minimized.

Owner

syoyo commented Dec 9, 2017

Super cool! I'll review code and check the reason of build failure as soon as possible.

@tigrazone

This comment has been minimized.

tigrazone commented Dec 9, 2017

please wait. now I want to implement some ideas and then merge when I implement and say to you.
You can read my sources from last commits

@tigrazone

This comment has been minimized.

tigrazone commented Dec 9, 2017

Now I want to refactorize line read and processing of readed string - rewrite in c-style and minimize chars copy

@syoyo

This comment has been minimized.

Owner

syoyo commented Dec 9, 2017

Firstly, it uses C++11 feature(unordered_map). I want keep tinyobjloader master C++03 compilable, thus will create experimental or C++11 branch and merge the PR to there.

@tigrazone

This comment has been minimized.

tigrazone commented Dec 9, 2017

so I can use map :-)

tigrazone
@tigrazone

This comment has been minimized.

tigrazone commented Dec 9, 2017

map remake done!
works fast and ok

@tigrazone

This comment has been minimized.

tigrazone commented Dec 9, 2017

now I publish remake with hashed material_map.
from tokens or material.name is calculated hash val and use it as key.
for red-black trees int compares if faster then string compare.
x31 hash function is good for speed and stability

@syoyo

This comment has been minimized.

Owner

syoyo commented Dec 9, 2017

Cool! > map remake done!

@syoyo

This comment has been minimized.

Owner

syoyo commented Dec 9, 2017

I have merged PR into tigrazone-master branch, and there is one error in unit test.

-------------------------------------------------------------------------------
pbr
-------------------------------------------------------------------------------
tester.cc:366
...............................................................................

tester.cc:380: FAILED:
  REQUIRE( 0.2 == Approx(materials[0].roughness) )
with expansion:
  0.2 == Approx( 0.0 )

Can you please investigate the reason of the failure, @tigrazone ?

@tigrazone

This comment has been minimized.

tigrazone commented Dec 9, 2017

I dont know what a problem. need to read sources. I reply you

tigrazone
@tigrazone

This comment has been minimized.

tigrazone commented Dec 9, 2017

i fix errors. try to retest.
on my computer tests passed ok

@syoyo

This comment has been minimized.

Owner

syoyo commented Dec 10, 2017

When its ready to start merge? Is there further commits planned?

@tigrazone

This comment has been minimized.

tigrazone commented Dec 10, 2017

two days I need to check one idea. I say you when to start merge

@tigrazone

This comment has been minimized.

tigrazone commented Dec 11, 2017

My other ideas has no effect.
Please merge with my last commit from my repository

@syoyo

This comment has been minimized.

Owner

syoyo commented Dec 12, 2017

Merged into tigrazone branch.

It requires some amount of code refactoring, and also there is slight API change, thus I may merge this together with PR #151

@tigrazone

This comment has been minimized.

tigrazone commented Dec 12, 2017

ok i'm happy to read this!

@syoyo

This comment has been minimized.

Owner

syoyo commented Dec 14, 2017

I have merged tigrazone and #151 into devel branch(which will become future v2.0.0). I'll do code refactoring and API cleanup from now on.

@tigrazone

This comment has been minimized.

tigrazone commented Jan 20, 2018

hello! what's new?
when you make release?

@syoyo

This comment has been minimized.

Owner

syoyo commented Jan 20, 2018

when you make release?

I found it requires some amount of code refactoring, so I can not promise when its ready to be released.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment