Speed up trace mode

[Mihir Luthra]

On Mon, May 13, 2019 at 2:19 AM Clemens Lang <cal at macports.org> wrote:

> Hi Mihir,
>
> In my experience, it will make it much simpler to review the entire code
> if you expand your repository with a README file which gives a (textual)
> rough overview of how the code works, possibly with a link to the paper
> you implemented. I'd also advise extensively commenting your code to
> explain why you do things the way to implemented them, but from a quick
> glance, it seems you already did that to a certain extent.
>

https://github.com/MihirLuthra/sharedMemory

I made a readme for the repository just as you suggested. Although if it
lacks to be understandable, I will update it accordingly as you say. I have
made the code extensively commented now and refactored it to a nice extent
I guess.

Also when reading code I suggest reading “sharedmemory.h” first and then
“sharedmemory.c” will make the comments more understandable.

Although this is the first time I coded multi-threaded program, and now I
realise that it really takes long to remove a small bug. But now I belief
the code is bug free and works completely correct.
Two extra things which I plan to do is reference counting for munmap(2)
after expanding and utilisation of wasted space in file(which will drop the
memory usage to almost half of what it currently is).
I have planned for them and probably will do these two in 2-3 days more.


>
> You can further abstract using a separate SharedMemoryManager object per
> thread if you use the pthread API. See for example
> __darwintrace_sock_set and sock_key in darwintrace.h and darwintrace.c.
>
> This should allow you to write a get_memory_manager() function that will
> automatically give you a thread-specific SharedMemoryManager object or
> create one if none exists for the current thread yet.
>
>
I saw the code in darwintrace.c.
Probably yes, I can call insert() and inside it I could call
get_memory_manager() which checks thread specific storage for it and it
makes the code pretty clean too.
Is that the way you mean?

Also, this reminded me of one more thing, if I implement search() in
open.c, access.c etc. I need not call __darwintrace_setup() right?
Like is there any other important thing darwintrace calls do which I need
to do separately after search() or can I straight away skip to the end
according to search() results?


>
> It's probably a good idea to move the path normalization to a separate
> function, yes. Note that the current does not correctly deal with
> directory symlinks. For example, trace mode does not recognize that
>
>   /opt/local/libexec/qt5/include/QtCore/QByteArray
>
> is a file installed by the qt5-qtbase port, because
>
>   /opt/local/libexec/qt5/include/QtCore
>
> is a symbolic link to
>
>   /opt/local/libexec/qt5/lib/QtCore.framework/Versions/5/Headers
>
> Once path lookup is fast, we could actually modify the normalization
> code to also check folder symbolic links inside the MacPorts prefix
> against the database of ports.
>
> This would require intertwining the path lookup with path normalization.
> Keep that in mind for later, but don't do it now.
>
> Got it. Will do once this shared memory functionality is complete.

>
>
>
> On Wed, May 08, 2019 at 04:24:28PM +0530, Mihir Luthra wrote:
> > I figured out the right ways to use my files in code. Can you explain
> > me why is it causing these errors.[1]
> >
> > Just before __darwintrace_is_in_sandbox_check returns path permission,
> > I captured it in a bool variable and inserted the normal path in the
> > shared file. Each time it prints insertion successfull, so probably I
> > feel the extra code I inserted didn’t’ crash, but if you can give me
> > some idea what possibly could have gone wrong I maybe able to debug
> > it.
> >
> > [1] https://pastebin.com/1t7a0GJq
>
> :info:extract sip_copy_proc:
> mkdir(/opt/macports-test/var/macports/sip-workaround): No such file or
> directory
>
> We use this directory to create a copy of system binaries that we
> wouldn't be able to inject into because they have the magic
> "unmodifiable system file" bit introduced by Apple in 10.13. Normally,
> typing 'sudo make install' in your macports development copy should have
> created this directory (see doc/base.mtree.in), but that doesn't seem to
> have happened for you. You should be able to create the directory
> manually and chmod it to 1777.
>

Got it. That error is gone now.

>
>
> > 2) When I install copies with multiple names, i mean I tried with
> > prefixes macports-test2, macports-test3 but just as like I sent you
> > the log file last time, it actually reads hardcoded “macports-test”
> > somewhere which I am unable to understand.
>
> You'll have to re-run ./configure with a different value for --prefix
> for each different installation. If you want to install into multiple
> places, it's probably wise to have multiple copies of the source code as
> well, otherwise you'll end up recompiling unchanged files very often
> during development.
>
>
> > 4) What location I should choose for my temporary shared files in your
> > opinion? I currently specified the path to be /tmp/.sharedmemory
> >
> > Is there any better location within macports directories?
>
> A path similar to what we use for the trace mode socket might be a good
> choice. The template for that socket is
>
>   /tmp/macports-trace-XXXXXX
>
> see src/port1.0/porttrace.tcl:51
>

Got it. Will do the same.

Meanwhile till the time you review my code, I will see if I can get the
code right into base and add other features to it.

Thanks for the help,
Mihir
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.macports.org/pipermail/macports-dev/attachments/20190516/abc1f619/attachment.html>